Skip to content

fix: add context.Canceled checks to get-details and get-versions handlers#1345

Open
friendlygeorge wants to merge 1 commit into
modelcontextprotocol:mainfrom
friendlygeorge:fix/cancellation-checks-all-handlers
Open

fix: add context.Canceled checks to get-details and get-versions handlers#1345
friendlygeorge wants to merge 1 commit into
modelcontextprotocol:mainfrom
friendlygeorge:fix/cancellation-checks-all-handlers

Conversation

@friendlygeorge

Copy link
Copy Markdown

Summary

#1335 fixed the list-servers endpoint to return 499 instead of 500 when the client disconnects mid-request. But the get-server-details and get-server-versions handlers still use raw huma.Error500InternalServerError without checking for context.Canceled.

This PR applies the same cancellation check pattern to both remaining GET handlers, so all server endpoints consistently return 499 on client disconnect instead of logging a noisy 500 and triggering superfluous response.WriteHeader warnings.

Changes

  • internal/api/handlers/v0/servers.go: Added context.Canceled check before logging and returning 500 in get-server-details (line ~188) and get-server-versions (line ~218)
  • Pattern matches ListServersError in list_errors.go (already tested)

Why this matters

Without this fix, clients that disconnect during get-details or get-versions requests still produce:

  1. Error-level log lines for benign cancellations (noise in logs/alerts)
  2. superfluous response.WriteHeader warnings (huma tries to write to a closed socket)

The fix short-circuits before the error log and 500 response, returning 499 instead.

Test plan

  • Existing tests pass (go test ./internal/api/handlers/v0/...)
  • Manual: curl --max-time 0.05 http://localhost:8080/v0/servers/<name>/versions/latest no longer logs error-level cancellation

The inline context.Canceled checks in get-server-details and
get-server-versions handlers pushed RegisterServersEndpoints cyclomatic
complexity to 24 (max 20). Extract a shared RegistryError helper that
handles context.Canceled, logging, and 500 responses, reducing
complexity back under the limit.

Also adds context.Canceled checks to both handlers (matching the
pattern already used in ListServersError for the list handler).
@friendlygeorge friendlygeorge force-pushed the fix/cancellation-checks-all-handlers branch from 2251a47 to f0ce5a8 Compare June 9, 2026 12:41
@friendlygeorge

Copy link
Copy Markdown
Author

Hi! This PR adds context.Canceled checks to the get-details and get-versions handlers. When the context is canceled (e.g., user interrupts), the handlers now return cleanly instead of proceeding with stale state.

This has been open for 5 days — let me know if there's anything you'd like me to adjust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant