Repository navigation
Conversation
Contributor
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 18 changed, 0 removedBuild ID: 9bcc0c4604f539289506dfbf URL: https://www.apollographql.com/docs/deploy-preview/9bcc0c4604f539289506dfbf
|
A failing Rhai script used to have its error reported verbatim to the
client, wrapped in the router's own text. That disclosed that the router
runs Rhai, the names of the script's callbacks, and the line and position
of the failure:
"rhai execution error: 'Runtime error: Invalid request (line 25, position 39)
in call to function 'process_router_request' @ 'process_router_request'
(line 6, position 29)'"
Clients now receive only a message the script author chose. A thrown string
is returned as written, and a thrown object map still picks the status code
and the full GraphQL response body. Everything else is replaced with the
status code's reason phrase and the real error is logged at ERROR level
instead, which covers:
- Failures raised by the Rhai engine itself, such as an undefined function
or a type mismatch.
- A throw carrying only a status: `throw #{ status: 400 }` now reads
`Bad Request` rather than dumping the thrown map.
- Failures in the router's own Rhai functions that raise no message, such
as reading a header that isn't there.
- A throw the router cannot read as a message - a value that is not a
string or an object map, or a map with an unreadable field, which is
discarded whole so any message beside the bad field goes with it.
`ErrorDetails` grows an `internal_detail` field holding the unredacted Rhai
error for the logs. It is skipped by serde so a value deserialized from a
script's `throw` can never populate it, and it is never copied into a
client-facing response.
Response-stage failures are logged outside the span the request macros
install, so they never recorded which stage failed. Now that the client
message is redacted, that was the missing half of the only record: they
carry `rhai.stage`, and they say `map_response` rather than `map_request`.
The empty-response-stream failure is redacted and logged the same way.
Known limitation, documented in the Rhai customization docs: a router
function that raises a message of its own still returns that message. A
router function's error is indistinguishable from a script's `throw` - both
arrive as `ErrorRuntime` carrying a string - so the discrimination is on the
value, not the origin: `engine::NO_CLIENT_MESSAGE` is redacted, anything
else is returned. `env::get()` on an unset variable and `json::decode()` on
malformed input therefore still reach the client; catch them and throw your
own error on a client-facing path.
Upgrade note: client-facing messages for a thrown string no longer include
the `rhai execution error: 'Runtime error: ... (line N, position M)'`
wrapper - only the string thrown. Nothing changes for scripts themselves: a
`catch` block receives exactly what it did before, and status codes are
unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rohan-b99
force-pushed
the
ROUTER-2050-redact-rhai-wrapper-from-client-errors
branch
from
August 13, 2026 16:38
af8703d to
487a337
Compare
2 of 10 tasks
Contributor
Author
|
/claude-review |
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
BrynCooke
requested changes
Oct 7, 2026
BrynCooke
left a comment
BrynCooke
left a comment
Contributor
There was a problem hiding this comment.
I think we need to handle all errors and not leak details.
The comments around the empty-response-stream detail and the reason-phrase fallback restated what the code and log line already say. Keep only the log-search prefix and the 599 fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test covers errors from the router's own Rhai functions in general, not only ones that carry no message, which the next commit makes the only kind. Rename it and its log snapshot to match; no change to what it asserts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The router's Rhai functions raised their errors as plain strings, the same
shape as a script's own `throw "..."`, so their text reached clients. Any
caller could learn which environment variables a script reads
(`env::get()` on an unset variable), or trigger parser errors from
`base64::decode()` and `json::decode()` on a client header.
Those functions now raise a `RouterFunctionError`. `process_error`
recognises it before the thrown-string branch: the client gets the status
code's reason phrase and the full text is logged. A thrown string that the
script chose still reaches the client unchanged.
Rhai prints a custom type by its type name, so the error is swapped for its
text before it is logged, and the script `log_*` functions do the same.
`to_string` and `to_debug` are registered so `${err}` in a script shows the
text.
This replaces `NO_CLIENT_MESSAGE`. A missing header now raises a message that
names the header, since that text no longer reaches the client.
A `catch` block that catches one of these errors now receives an error
object instead of a string. `${err}` still gives the message, but
`type_of(err)` and comparisons such as `err == "..."` change. The docs and
changeset call this out.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rohan-b99
changed the title
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
2 participants
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.
What it does
A failing Rhai script used to have its error reported verbatim to the client, wrapped in the router's own text. That disclosed that the router runs Rhai, the names of the script's callbacks, and the line and position of the failure:
Clients now receive only a message the script author chose. A thrown string is returned as written, and a thrown object map still picks the status code and the full GraphQL response body. Everything else is replaced with the status code's reason phrase and the real error is logged at ERROR level instead, which covers:
throw #{ status: 400 }now readsBad Requestrather than dumping the thrown map.env::get()on an unset variable,base64::decode()orjson::decode()on malformed input, or reading a header that isn't there.ErrorDetailsgrows aninternal_detailfield holding the unredacted Rhai error for the logs. It is skipped by serde so a value deserialized from a script'sthrowcan never populate it, and it is never copied into a client-facing response.Response-stage failures are logged outside the span the request macros install, so they never recorded which stage failed. Now that the client message is redacted, the log is the only record: they carry
rhai.stage, and they saymap_responserather thanmap_request. The empty-response-stream failure is redacted and logged the same way.Why
The engine's error text describes the script's internals, and the router's own Rhai functions describe the script's inputs. They can name the environment variables a script reads, or echo a parser error for a client-supplied header. None of that is a message the script author chose to show a client.
A router function's error used to be a plain string, the same shape as a script's own
throw "...", so nothing could tell the two apart. The router's Rhai functions now raise aRouterFunctionErrorinstead, andprocess_errorchecks for it before the thrown-string branch. That lets a script's own string reach the client while a function's error text does not. Rhai prints a custom type by its type name, so the error is swapped for its text before it is logged, and the scriptlog_*functions do the same.to_stringandto_debugare registered so${err}shows the text in scripts.Upgrade notes
rhai execution error: 'Runtime error: ... (line N, position M)'wrapper, only the string thrown. Status codes are unchanged.catchblock that catches an error from one of the router's Rhai functions now receives an error object instead of a string.${err}still gives the message, buttype_of(err)and comparisons such aserr == "..."change.Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
A lot of (if not most) features benefit from built-in observability and
debug-level logs. Please read this guidance on metrics best-practices. ↩Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩