Improve task status API response structure and consistency#96
Improve task status API response structure and consistency#96Darkshadow0409 wants to merge 1 commit intoindictechcom:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the /api/task_status/<task_id> endpoint in app.py to return a more structured, backend-consistent response shape for Celery task polling.
Changes:
- Introduces top-level
success,data, anderrorsfields in the task status response. - Renames the task status field from
statustostate. - Returns task failure messages via a structured
errorslist.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response = { | ||
| "success": True, | ||
| "task_id": task_id, | ||
| "status": task.status, | ||
| "result": task.result if task.successful() else None, | ||
| "state": task.status, # PENDING, STARTED, SUCCESS, FAILURE | ||
| "data": {}, | ||
| "errors": [] | ||
| } |
There was a problem hiding this comment.
This endpoint’s response schema is a breaking change (renames status→state, result→data, and error→errors). The current frontend polling code still destructures { status, result, error } from /api/task_status (see frontend/src/pages/Upload.js), so this change will break uploads unless the client is updated in the same PR or the API supports the legacy fields during a transition.
| if task.successful(): | ||
| response["data"] = task.result | ||
|
|
||
| # If task failed, include error information | ||
| if task.failed(): | ||
| response["error"] = str(task.result) | ||
| elif task.failed(): | ||
| response["success"] = False | ||
| response["errors"] = [str(task.result)] |
There was a problem hiding this comment.
task.successful() can be True even when the task result represents an application-level failure (e.g., tasks.py returns { "success": False, "errors": [...] } on KeyError without raising, so Celery marks the task as SUCCESS). In that case this endpoint will return top-level success: True and errors: [], which contradicts the “structured error handling” goal. Consider normalizing: if task.result is a dict with success == False / errors, propagate those to the top-level response (or make the Celery task raise on failure instead of returning a failure payload).
| "task_id": task_id, | ||
| "status": task.status, | ||
| "result": task.result if task.successful() else None, | ||
| "state": task.status, # PENDING, STARTED, SUCCESS, FAILURE |
There was a problem hiding this comment.
The inline state comment is incomplete: tasks.py explicitly sets state='PROGRESS' via update_state, so clients may see PROGRESS here as well. Update the comment (and any client expectations/docs) to reflect all states this API can return.
| "state": task.status, # PENDING, STARTED, SUCCESS, FAILURE | |
| "state": task.status, # Celery task state (e.g. PENDING, STARTED, PROGRESS, SUCCESS, FAILURE) |
| # Improve Task Status API | ||
| response = { | ||
| "success": True, | ||
| "task_id": task_id, | ||
| "status": task.status, | ||
| "result": task.result if task.successful() else None, | ||
| "state": task.status, # PENDING, STARTED, SUCCESS, FAILURE | ||
| "data": {}, | ||
| "errors": [] | ||
| } |
There was a problem hiding this comment.
PR description claims this change “ensured uniform API response format across endpoints”, but this file still has endpoints returning error (singular) instead of errors (plural) (e.g., /api/preference and /api/user_language GET responses). Either broaden this PR to update those responses too, or adjust the PR description to reflect that only /api/task_status is being changed.
Problem
The current task status API returns responses in a format that is inconsistent with other backend endpoints. It uses fields like "status" and "result" without clearly separating success, data, and error states.
Solution
This PR improves the response structure of the task status API to make it consistent with other endpoints.
Changes made:
Benefits