Skip to content

fix(api-gateway): preserve gRPC error semantics instead of collapsing to HTTP 500 - #2

Open
pranav8764 wants to merge 2 commits into
Golden-Incodent-Corpusfrom
fix/gateway-grpc-error-mapping
Open

pranav8764 wants to merge 2 commits into
Golden-Incodent-Corpusfrom
fix/gateway-grpc-error-mapping

Conversation

@pranav8764

@pranav8764 pranav8764 commented Aug 2, 2026 •

Copy link
Copy Markdown
Owner

Problem

A repository added through the archiatros-sre GitHub App never appears in the frontend. One reason the bug stayed invisible: the API gateway collapsed every gRPC error from the Integration Service into an indistinguishable HTTP 500 (or 502), so the browser could not tell "this installation belongs to a different GitHub App, reconnect it" from "the database is down".

api-gateway: "list repositories failed","error":"rpc error: code = Internal desc = failed to authorize with github"
api-gateway: GET /api/v1/workspaces/.../integrations/github/installations/.../repositories status=500

Change

Adds a shared resolveGRPCError / writeGRPCError pair in services/api-gateway/internal/handler/integration.go that maps the gRPC status codes the Integration Service uses deliberately onto real HTTP statuses and stable, machine-readable error codes:

gRPC code HTTP error code
FailedPrecondition 409 INSTALLATION_REVOKED
NotFound 404 NOT_FOUND
PermissionDenied 403 FORBIDDEN
Unauthenticated 401 UNAUTHORIZED
InvalidArgument 400 INVALID_REQUEST
Unavailable 503 UPSTREAM_UNAVAILABLE
anything else each handler's existing default each handler's existing code

Applied to eight handlers: CreateGitHubInstallSession, ListConnectedRepositories, GitHubConfig, GitHubCallback, ListInstallations, ListRepositories, AttachRepository, and GetRepository.

ListConnectedRepositories is the one that closes the loop on the user's report — it backs the dashboard's repository list, on both its workspace-scoped and cross-workspace routes. Paired with #4, a connection orphaned by GitHub App rotation now reaches the browser as an actionable 409 INSTALLATION_REVOKED instead of an opaque 502.

Notes

  • No behaviour regressions. Unmapped codes fall through to each handler's pre-existing status and error-code string, so CreateGitHubInstallSession and ListConnectedRepositories still default to 502 and the rest still default to 500.
  • Independently mergeable. This is a mapping only, in no particular order with feat(db): bind GitHub installations to their App, allow revoked repo connections #3/fix(integration): detect GitHub App rotation, and revoke what it orphaned #4/fix(frontend): show why a repository never appeared, not an empty dropdown #5. It is correct whether or not the Integration Service already returns codes.FailedPrecondition for a foreign-App installation.
  • No credential leakage. Every mapped code carries a fixed message of our own. Upstream error text can embed installation tokens, webhook signatures, or raw GitHub response bodies, so err.Error() is never forwarded to the client. The existing structured slog logging at each call site is unchanged.
  • Uses the existing writeError / writeJSON envelope; no new response shape.
  • Deliberately left alone: UpsertLogProvider (400), ListLogProviders (502), GitHubWebhook (401), and DecideFixApproval (409) keep their hand-picked statuses. They already return FailedPrecondition for unrelated conditions ("approval has expired", "provider credential encryption is not configured"), which the INSTALLATION_REVOKED label would misdescribe.

Testing

Table-driven tests in services/api-gateway/internal/handler/integration_test.go cover every mapped gRPC code, the default fall-through for all eight handlers, FailedPrecondition -> 409 across all eight, both ListConnectedRepositories routes, wrapped status errors, and plain non-status errors. Each case also asserts that no upstream text (ghs_secret_token, a postgres:// DSN, rpc error) reaches the response body.

cd services/api-gateway && go vet ./... && go test ./...   # pass
make test                                                  # all five Go services pass

make test also runs the Python suites; ai-worker fails locally with No module named pytest, a pre-existing environment gap unrelated to this change.

Browser e2e was intentionally skipped here — #5 owns that flow and only one stack exists on the dev machine.

🤖 Generated with Claude Code

pranav8764 and others added 2 commits August 2, 2026 20:17
Every gRPC error from the Integration Service collapsed into an identical
HTTP 500, so the browser could not distinguish "this installation belongs
to a different GitHub App, reconnect it" from "the database is down". A
repository added through a stale GitHub App therefore failed silently.

Add a shared resolveGRPCError/writeGRPCError pair that maps deliberate
gRPC codes onto real HTTP statuses and stable, machine-readable error
codes:

  FailedPrecondition -> 409 INSTALLATION_REVOKED
  NotFound           -> 404 NOT_FOUND
  PermissionDenied   -> 403 FORBIDDEN
  Unauthenticated    -> 401 UNAUTHORIZED
  InvalidArgument    -> 400 INVALID_REQUEST
  Unavailable        -> 503 UPSTREAM_UNAVAILABLE

Anything else falls through to each handler's pre-existing default status
and error code, so nothing that already worked changes shape — including
CreateGitHubInstallSession, whose default stays 502.

Mapped codes carry fixed messages of our own. Upstream error text may
embed installation tokens, webhook signatures, or raw GitHub response
bodies, so err.Error() is never forwarded to the client; the existing
structured slog logging is untouched.

Applied to CreateGitHubInstallSession, GitHubConfig, GitHubCallback,
ListInstallations, ListRepositories, AttachRepository, and GetRepository.
This is a mapping only and does not depend on the Integration Service
emitting FailedPrecondition yet.

Covered by table-driven tests over every mapped code, the default
fall-through per handler, wrapped status errors, and non-status errors,
each asserting no upstream text reaches the response body.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ListConnectedRepositories backs the dashboard's repository list and is the
exact path in the original bug report, but it collapsed every gRPC error
into an opaque 502. With the Integration Service now returning
codes.FailedPrecondition when an installation belongs to a different
GitHub App, that reconnect signal never reached the browser.

Route it through the same writeGRPCError mapping as the other handlers,
keeping 502 / REPOSITORY_LIST_FAILED as its fall-through default so
unclassified failures are unchanged.

Covered on both of its routes — workspace-scoped and cross-workspace —
across every mapped code and the fall-through.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pranav8764
pranav8764 force-pushed the fix/gateway-grpc-error-mapping branch from ad7e14d to a476333 Compare August 2, 2026 14:50
@pranav8764
pranav8764 changed the base branch from main to Golden-Incodent-Corpus August 2, 2026 14:51
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