git.iko.soy/crush
on main help sign in
chore(server,tests): cover async cancellation cleanup behavior
5aaa709Christian Rocha
internal/server/agent_cancel_test.go+30
compare with main
unifiedsplit
@@ -150,6 +150,36 @@ func TestPostAgent_ReturnsOKOnContextCanceled(t *testing.T) {
150 t.Fatal("dispatched run was never entered")
150 t.Fatal("dispatched run was never entered")
151 }
151 }
152 close(coord.release)
152 close(coord.release)
153+
154+ // Wait for the dispatched run to fully return. Backend.runAgent
155+ // swallows context.Canceled, so it must not publish a
156+ // notify.TypeAgentError. Publishing would dereference the synthetic
157+ // workspace's nil notification broker and crash this goroutine,
158+ // which is the explicit guard that a cancel produces no top-level
159+ // error event.
160+ require.Eventually(t, func() bool {
161+ return coord.ranCount.Load() == 1
162+ }, 2*time.Second, 10*time.Millisecond)
163+}
164+
165+// TestHandleError_ContextCanceledFallsThroughTo500 documents the step 8
166+// cleanup: the old context.Canceled special case in handleError was
167+// removed because runtime cancellation of an agent run can no longer
168+// reach handleError. The agent-prompt handler returns 202 before the run
169+// starts (fire-and-forget SendMessage) and Backend.runAgent swallows
170+// context.Canceled. Any context.Canceled that still reaches handleError
171+// is therefore an unexpected synchronous error and falls through to the
172+// default 500 like any other.
173+func TestHandleError_ContextCanceledFallsThroughTo500(t *testing.T) {
174+ t.Parallel()
175+
176+ c := &controllerV1{server: &Server{}}
177+ rec := httptest.NewRecorder()
178+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
179+
180+ c.handleError(rec, req, context.Canceled)
181+
182+ require.Equal(t, http.StatusInternalServerError, rec.Code)
153}
183}
154
184
155// TestPostAgent_DetachesRequestContext verifies that the dispatched run
185// TestPostAgent_DetachesRequestContext verifies that the dispatched run
internal/server/proto.go+8−9
@@ -1,7 +1,6 @@
1package server
1package server
2
2
3import (
3import (
4− "context"
5 "encoding/json"
4 "encoding/json"
6 "errors"
5 "errors"
7 "fmt"
6 "fmt"
@@ -1035,15 +1034,15 @@ func (c *controllerV1) handleGetWorkspacePermissionsSkip(w http.ResponseWriter,
1035
1034
1036// handleError maps backend errors to HTTP status codes and writes the
1035// handleError maps backend errors to HTTP status codes and writes the
1037// JSON error response.
1036// JSON error response.
1037+//
1038+// Runtime cancellation of an agent run no longer reaches here for the
1039+// agent-prompt path: SendMessage is fire-and-forget (the handler returns
1040+// 202 before the run starts) and Backend.runAgent swallows
1041+// context.Canceled, surfacing the FinishReasonCanceled marker to SSE
1042+// subscribers instead. The remaining callers pass synchronous backend
1043+// errors, so context.Canceled gets no special case and would fall through
1044+// to the default 500 like any other unexpected error.
1038func (c *controllerV1) handleError(w http.ResponseWriter, r *http.Request, err error) {
1045func (c *controllerV1) handleError(w http.ResponseWriter, r *http.Request, err error) {
1039− // A canceled agent run is not an error from the prompting
1040− // client's perspective. The cancellation reaches every SSE
1041− // subscriber via the FinishReasonCanceled marker on the assistant
1042− // message; the still-open POST should not surface a 500.
1043− if errors.Is(err, context.Canceled) {
1044− w.WriteHeader(http.StatusOK)
1045− return
1046− }
1047 status := http.StatusInternalServerError
1046 status := http.StatusInternalServerError
1048 switch {
1047 switch {
1049 case errors.Is(err, backend.ErrWorkspaceNotFound):
1048 case errors.Is(err, backend.ErrWorkspaceNotFound):