Skip to content

feat(server): answer every API error with a JSON body - #6916

Merged
otavio merged 2 commits into
masterfrom
feat/api-error-body
Aug 18, 2026
Merged

feat(server): answer every API error with a JSON body#6916
otavio merged 2 commits into
masterfrom
feat/api-error-body

Conversation

@otavio

@otavio otavio commented Aug 18, 2026

Copy link
Copy Markdown
Member

What

Every API error response now carries a JSON body: {"message": …}, plus {"fields": …} when the
error names the request fields it concerns. The console stopped rendering server prose and shows
its own copy, keyed by status.

Why

Every 4xx and 5xx returned an empty body. A client learned that something failed but never what.
Nine error banners in the console rendered the server's message and therefore rendered nothing.
Eight handlers worked around the gap by building their own bodies, producing three different shapes
for the same kind of failure. The OpenAPI specification declared a message body for 401 on 109
operations and for 500 on 137, and the server sent none of them.

Closes #6915

Changes

  • server/api/pkg/echo/handlers: the HTTP error handler renders the body on all four exit paths.
    A 5xx answers with a fixed generic message — the reporter still receives the real one, because
    those texts describe internals. An error carrying an HTTP status reads its Message member
    rather than Error(), which embeds the internal cause; *echo.BindingError is matched
    explicitly because it unwraps to its cause rather than to the *echo.HTTPError it embeds.
    The layer switch gains a default arm: the scope package declares a layer the switch does not
    name, which previously produced status zero.
  • server/api/routes: the eight handlers that built their own bodies return the error instead.
    RegisterUser, UpdateUser, CreateTag and UpdateTag carry the conflicting field names on
    the error and dropped the extra return value. NewErrDuplicated builds the field map, so cloud's
    admin duplicates gain bodies with no cloud-side change.
  • server/api/services: UpdateTag reported a TagConflicts store failure as a 409 conflict.
    It now returns the store error, matching CreateTag. This is the one status this PR changes.
  • openapi: the shared 4xx and 5xx responses reference a new apiError schema. The three
    operations whose body shape changed point at the shared 400 and 409. The admin paths keep
    invalidFields/conflictFields, because cloud's admin handlers still emit those arrays.
  • ui/apps/console: apiErrorMessage and apiErrorFields in src/api/errors.ts. The nine
    banners call the first. Sign-up and accept-invite read the field map through the second, so they
    keep per-field highlighting. Neither reads the server's message: that member is for API
    clients, and reads as internal phrasing in the console.

Testing

The behaviour is pinned at two seams: the table in
server/api/pkg/echo/handlers/errors_test.go (19 cases, including the binding-error leak, an
unrecognised layer, and the two paths that must not echo their own message) and
ui/apps/console/src/api/__tests__/errors.test.ts.

Worth probing:

  • GET /api/devices?page=abc — a binding error must answer {"message":"failed to bind field value to int"}, never the strconv.ParseInt cause.
  • server/api/routes/nsadm_test.go guards that a 403 body leaks neither a namespace owner's nor a
    member's email. It is the reason the module renders the error's own message and never resource
    detail.
  • The message member has no console consumer by design. It exists for API clients and for the
    specification's existing promise — it is not dead code.

Full suites pass: shellhub server (27 packages), cloud with -tags enterprise (22 packages), and
console (211 files). golangci-lint, the console lint and build, redocly lint and prettier are all
clean, and go mod tidy leaves both modules unchanged.

otavio added 2 commits August 18, 2026 09:06
Every 4xx and 5xx carried an empty body, so a client learned that something failed but never
what. The HTTP error handler now renders {"message": …}, plus {"fields": …} when the error names
the request fields it concerns.

A 5xx never echoes the error's own text: those messages describe internals, and only the reporter
sees them. Errors carrying an HTTP status read their message member rather than Error(), which
embeds the internal cause.

The eight handlers that hand-built their own bodies return the error instead, so the three ad-hoc
shapes become one. Four service methods move their conflicting field names onto the error and drop
the extra return value.

Fixes: #6915
Nine banners rendered the server's message, which was always empty, so a user who hit an error on
the devices, containers, or sessions list, or on any of six admin pages, saw a blank banner.

`apiErrorMessage` turns a caught value into display text keyed by status. It deliberately ignores
the server's own message member: that text is for API clients and reads as internal phrasing here.
`apiErrorFields` reads the per-field detail, which sign-up and accept-invite use to mark the field
that is already taken.
@otavio
otavio requested review from a team as code owners August 18, 2026 12:07
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Claude finished @otavio's task in 5m 3s —— View job


Code Review Complete

Reviewed 57 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

A few things worth noting as positives from the review:

  • 5xx never leaks internals. respond() unconditionally overwrites message, fields = genericMessage, nil for any status >= 500 before writing the body, while Sentry still receives the real cause. The binding-error path correctly returns the framework's safe "failed to bind field value to int" rather than the strconv.ParseInt cause — the *echo.BindingError-before-*echo.HTTPError ordering in echoMessage is necessary and correct, since BindingError promotes HTTPError.Unwrap() and would otherwise walk straight past to the internal cause.
  • fieldsOf only surfaces safe maps. It exposes ErrDataInvalidEntity.Fields / services.ErrDataInvalidFields.Fields (field-name → fixed reason) and deliberately leaves ErrDataInvalid.Data (which can hold real field values) unrendered. No email/ownership detail reaches 403/404 bodies.
  • Signature changes are consistent. Dropping the []string conflicts return from RegisterUser/UpdateUser/CreateTag/UpdateTag is applied uniformly across services, interface docs, mock_service.go, and every route/service test. The old UpdateTag latent bug (returning NewErrTagDuplicated even on a bare store error) is incidentally fixed by the new if !has { return err } guard, and covered by a new test case.
  • Cloud is unaffected. No stale call sites — cloud's admin UpdateUser/RegisterUser operate on distinct cloud request types and its own interfaces; the removed ErrDataDuplicated/NewErrDuplicated/NewErrTagDuplicated symbols have zero references in cloud/, and the admin conflict endpoints emit their own []string bodies untouched by this handler.
  • UI is sound. apiErrorMessage/apiErrorFields are well-typed with no any, defensively guard non-object/array fields, and all render sites use JSX text interpolation (React-escaped). The signUpStore switch to Object.keys(apiErrorFields(error)) preserves the field-detection behavior.

To request another review round, comment /review.

@otavio
otavio merged commit f82a265 into master Aug 18, 2026
42 checks passed
@otavio
otavio deleted the feat/api-error-body branch August 18, 2026 15:56
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.

API error responses carry a JSON body

2 participants