Fix/1158 task lock not found #1168
Conversation
bytecii
left a comment
bytecii
left a comment
There was a problem hiding this comment.
Maybe we can also add some unit tests for this. Thanks!
| question: tempMessageContent, | ||
| task_id: nextTaskId, | ||
| attaches: improveAttaches, | ||
| }); |
There was a problem hiding this comment.
Should we do something like this?
if (res?.code === 410) {
// Re-establish SSE + task lock via POST /chat
await chatStore.startTask(nextTaskId, ...);
}
There was a problem hiding this comment.
I think we need that because when the improve endpoint returns 410, we fall back to startTask() which reestablishes both the SSE consumer and the task lock.
darkmoon-o
force-pushed
the
fix/1158-task-lock-not-found
branch
from
ea25ee1 to
4cc1802
Compare
Yep, for sure. I added test cases for this. |
| def supplement(id: str, data: SupplementChat): | ||
| chat_logger.info("Chat supplement requested", extra={"task_id": id}) | ||
| task_lock = get_task_lock(id) |
There was a problem hiding this comment.
Why we still use get_task_lock in supplement, stop, add_task
There was a problem hiding this comment.
We keep get_task_lock() on supplement/stop/add_task because they only make sense with an existing active session; creating a new lock there would silently drop the action (no SSE consumer). improve is user-facing, so we return 410 and let the client reconnect via POST /chat to re-create both lock + SSE.
darkmoon-o
force-pushed
the
fix/1158-task-lock-not-found
branch
from
504dea0 to
479c5e1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| "Chat improvement requested", | ||
| extra={"task_id": id, "question_length": len(data.question)}, | ||
| ) | ||
| task_lock = get_task_lock(id) | ||
| task_lock = get_task_lock_if_exists(id) | ||
|
|
||
| if task_lock is None: | ||
| # SSE session no longer exists (disconnected, timed out, or stopped). | ||
| # Return 410 Gone so the frontend can fall back to POST /chat which | ||
| # creates both a new task lock AND a new SSE consumer. | ||
| chat_logger.warning( | ||
| "Task lock not found for improve request, " | ||
| "returning 410 so client reconnects via POST /chat", | ||
| extra={"project_id": id}, | ||
| ) |
There was a problem hiding this comment.
258 line extra={"task_id": id
but 269 line extra={"project_id": id
this id should be task_id right?
| fetchPost(`/chat/${projectStore.activeProjectId}`, { | ||
| question: tempMessageContent, | ||
| task_id: nextTaskId, | ||
| attaches: improveAttaches, | ||
| }); |
There was a problem hiding this comment.
The backend now returns 410 when the task lock is missing, but currently there's no frontend handling for it. The existing fetchPost call in ChatBox/index.tsx is fire-and-forget (no await, no .then()/.catch()), and handleResponse in src/api/http.ts doesn't check HTTP status codes — so the 410 response gets silently discarded. Could we add corresponding frontend handling (e.g. await the response, detect 410, and show a reconnect prompt to the user) to make this fix actually take effect?
darkmoon-o
force-pushed
the
fix/1158-task-lock-not-found
branch
from
b9b44d5 to
343b11c
Compare
4 participants
Description
Fixes “task lock not found” errors by making lock acquisition/release idempotent and improving retry handling. This prevents failed runs when a lock record is missing or already released.
What is the purpose of this pull request?