Skip to content

web: show the real message for Relay's generic field-error throws - #154

Open
sdunster wants to merge 1 commit into
pr131from
pr153
Open

web: show the real message for Relay's generic field-error throws#154
sdunster wants to merge 1 commit into
pr131from
pr153

Conversation

@sdunster

@sdunster sdunster commented Aug 21, 2026

Copy link
Copy Markdown
Member

Relay's own thrown-error text for every field-level failure kind
except client-side Relay Resolvers is a hardcoded, generic string —
even relay_field_payload.error, fired for an actual server-reported
GraphQL error on a queried field, throws "Unexpected response payload

  • check server logs for details." and never includes the real
    message (relay-runtime's handlePotentialSnapshotErrors.js). Confirmed
    against a real case: a period referencing a deleted category makes
    CategoryPeriodSummary.category (non-null) fail, collapsing the whole
    ActivityTotalsDisplayQuery response — but the query selects
    periodSummaryByMember before periodSummaryByCategory, so Relay's
    read-time "missing expected data" check reports the earlier,
    unrelated field first, and that event carries no message field at
    all. The user sees "Missing expected data at path
    'location.periodSummaryByMember'" with no way to tell it was actually
    a bad category.

relayFieldLogger already receives the real message on relay_field_
payload.error/relay_resolver.error events, synchronously, moments
before the generic throw. It now buffers those (capped, cleared on
read) and exposes takeRecentFieldErrorMessages(). describeCaughtError
uses that to substitute the real message whenever the caught error's
own text is one of Relay's generic "Relay: ..." strings, leaving every
other error (including the "No data returned..." whole-query failure,
which already embeds the real text itself) untouched.

That buffer-read is a destructive side effect — it drains on read, so
it's safe to call exactly once per real catch, but not from
render-phase code: this app has React's StrictMode enabled
(main.tsx), which deliberately double-invokes things like a useState
lazy initializer specifically to catch impurity like this. A first
(thrown-away) invocation would drain the real message and a second,
committed one would find the buffer already empty. componentDidCatch
— react-error-boundary's onError — is a real lifecycle method
instead, and React guarantees it runs exactly once per catch even
under StrictMode. A new useCaughtErrorMessage hook resolves the
message there and stores it in state; PageErrorFallback takes the
resolved message as a prop rather than computing it itself, falling
back to the raw error's own message for the brief window before
onError has run (in practice invisible, since the text sits behind a
collapsed "Show details" until then).

RelayErrorBoundary adopts the hook directly. The three plain
ErrorBoundary + PageErrorFallback pairings that had no room to plumb a
resolved message through FallbackComponent (Router.tsx's root, home/
Layout.tsx, admin/Layout.tsx's pre-environment shell) move to a new
PageErrorBoundary, which pairs the same hook with a plain
resetErrorBoundary (no Relay store/retry machinery needed for what it
wraps).

RelayErrorBoundary.test.tsx and the new PageErrorBoundary.test.tsx each
add a test that renders under and asserts the real
buffered message appears — verified against a naive render-phase read
that it actually fails there, not just that it passes with the
onError-based approach.


Stack created with Sapling. Best reviewed with ReviewStack.

Relay's own thrown-error text for every field-level failure kind
except client-side Relay Resolvers is a hardcoded, generic string —
even relay_field_payload.error, fired for an actual server-reported
GraphQL error on a queried field, throws "Unexpected response payload
- check server logs for details." and never includes the real
message (relay-runtime's handlePotentialSnapshotErrors.js). Confirmed
against a real case: a period referencing a deleted category makes
CategoryPeriodSummary.category (non-null) fail, collapsing the whole
ActivityTotalsDisplayQuery response — but the query selects
periodSummaryByMember before periodSummaryByCategory, so Relay's
read-time "missing expected data" check reports the earlier,
unrelated field first, and that event carries no message field at
all. The user sees "Missing expected data at path
'location.periodSummaryByMember'" with no way to tell it was actually
a bad category.

relayFieldLogger already receives the real message on relay_field_
payload.error/relay_resolver.error events, synchronously, moments
before the generic throw. It now buffers those (capped, cleared on
read) and exposes takeRecentFieldErrorMessages(). describeCaughtError
uses that to substitute the real message whenever the caught error's
own text is one of Relay's generic "Relay: ..." strings, leaving every
other error (including the "No data returned..." whole-query failure,
which already embeds the real text itself) untouched.

That buffer-read is a destructive side effect — it drains on read, so
it's safe to call exactly once per real catch, but not from
render-phase code: this app has React's StrictMode enabled
(main.tsx), which deliberately double-invokes things like a useState
lazy initializer specifically to catch impurity like this. A first
(thrown-away) invocation would drain the real message and a second,
committed one would find the buffer already empty. componentDidCatch
— react-error-boundary's onError — is a real lifecycle method
instead, and React guarantees it runs exactly once per catch even
under StrictMode. A new useCaughtErrorMessage hook resolves the
message there and stores it in state; PageErrorFallback takes the
resolved message as a prop rather than computing it itself, falling
back to the raw error's own message for the brief window before
onError has run (in practice invisible, since the text sits behind a
collapsed "Show details" until then).

RelayErrorBoundary adopts the hook directly. The three plain
ErrorBoundary + PageErrorFallback pairings that had no room to plumb a
resolved message through FallbackComponent (Router.tsx's root, home/
Layout.tsx, admin/Layout.tsx's pre-environment shell) move to a new
PageErrorBoundary, which pairs the same hook with a plain
resetErrorBoundary (no Relay store/retry machinery needed for what it
wraps).

RelayErrorBoundary.test.tsx and the new PageErrorBoundary.test.tsx each
add a test that renders under <StrictMode> and asserts the real
buffered message appears — verified against a naive render-phase read
that it actually fails there, not just that it passes with the
onError-based approach.
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