fix: return 4xx for client errors on canvas repository file endpoint#5245
Draft
cursor[bot] wants to merge 1 commit into
Draft
fix: return 4xx for client errors on canvas repository file endpoint#5245cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
Previously, the public /api/v1/canvases/{canvas_id}/repository/file
endpoint mapped every error from ReadRepositorySpecFile to HTTP 500.
That meant client-caused errors -- canvas without a live version
(NotFound), draft owned by another user (PermissionDenied), invalid
version_id query string (InvalidArgument) -- all surfaced as 500
responses and got captured by the public middleware as Sentry HTTP
errors.
Map the gRPC status code carried by ReadRepositorySpecFile to a
matching 4xx response so only true server errors (Internal/Unknown)
are returned as 500, and only those reach Sentry. Pass the gRPC
message through for client errors so the UI/CLI can surface a useful
hint, and keep error-level logging only for actual 5xx responses.
|
👋 Commands for maintainers:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Sentry surfaced repeated
HTTP 500 /api/v1/canvases/{canvas_id}/repository/fileerrors from production(issue 7539528545).
The public download handler (
pkg/public/repository_file_download.go) wrapscanvases.ReadRepositorySpecFileand treats every error from it as an internal server error, even though that helper returns proper gRPC status codes:codes.NotFound— canvas live version missing, version not found, change request not foundcodes.PermissionDenied— draft owned by another user, version not visible in current flowcodes.Unauthenticated— no user in contextcodes.InvalidArgument— badorganization_id, badcanvas_id, badversion_id, unsupported spec pathAll of these are client-caused but were being mapped to
HTTP 500. The logging middleware then forwarded each one to Sentry viahub.CaptureMessage(...), which usessentry.LevelInfo— exactly matching the level/title shape we saw in the Sentry issue (HTTP 500 ..., levelinfo).Fix
Map the gRPC status code carried by
ReadRepositorySpecFileback to the matching HTTP status:NotFoundPermissionDeniedUnauthenticatedInvalidArgument/FailedPrecondition/OutOfRangeClient errors now pass the gRPC
Message()through to the response body so the UI/CLI can surface a useful hint, and they are logged at debug level instead of error so they don't pollute the server logs.The git-backed file path (
gitProvider.GetFile) is unchanged: those errors already log"Failed to get file"and return 500, and the existing test asserts that mapping for missing files.Tests
Test__RepositoryFileDownloadcases covering canvas.yaml / console.yaml success and the 400 / 404 mapping for invalid / missingversion_id(require the dev DB).TestRepositorySpecFileHTTPStatusthat exercises the helper directly without a database, covering all mapped codes plus a non-gRPC error fallback.Verification
go build ./pkg/public/... ./pkg/grpc/actions/canvases/...✅go vet ./pkg/public/...✅go test -run TestRepositorySpecFileHTTPStatus ./pkg/public/✅gofmt -lclean ✅