fix: contain async loggers and redact real preview tokens - #7
Merged
Conversation
Four findings from the GPT-5.6 Sol review of the typed-error-codes work. safeLogger only wrapped a synchronous throw. `Logger` declares `void`, and TypeScript accepts an `async` method there, so a log shipper whose methods reject produced an unhandled rejection — the runtime ends the process for that, mid-handoff, before the relay sandbox is released. The wrapper now attaches a handler to the runtime return value in the same tick it is created; a throwing property getter was already covered and now has a test. Redaction guessed the credential's grammar as `pt_...`, but the preview token is a ~362-character JWT (docs/measurements/01-preview-transport.md §3), so a real one survived both `pt_token%3D<jwt>` and a bare `invalid preview token <jwt>`. `waitForHealth` now takes the exact token out of the URL it is polling and redacts that value and its percent-encoded form by comparison; the pattern rules stay as the net for text where the value is not known, with a JWT-shaped rule added. The test fixture is a JWT. The SDK error attached as `cause` kept the unredacted body and message, which every error serialiser prints. It is now a clone with the same prototype, `name`, `status` and `code`, and with `message` and the parsed `body` redacted; the same for the error `killSandbox` surfaces. `browser.isConnected()` ran outside `checkedPage`'s try, so a browser proxy whose liveness accessor throws escaped as a plain `Error` instead of `browser_unusable`. It is read inside the try and branched on after. Also: the health poll no longer issues a final request with no budget left, whose abort used to overwrite the proxy's own answer — the one useful thing in `relay_not_ready`'s message — with "The operation timed out". The CHANGELOG bullets Sol called overstated now say what is true. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…oded dots Round 1 on the verification pass of PR #7. The clone was built with `Object.assign`, which copies own *enumerable* properties only. `new Error(msg, { cause })` installs `cause` non-enumerable, so an error chain was truncated exactly where the root reason lives — undici's `TypeError: fetch failed` is that shape. The copy now starts from `Object.getOwnPropertyDescriptors`, so every own property survives with its descriptor, and `message`/`stack`/`body` are redefined rather than assigned, which keeps them off `JSON.stringify(cause)` as they are on a real Error. The nested `cause` is redacted recursively, bounded at eight links and cycle-safe. Reading a foreign error is running foreign code, and this all happens inside the `catch` that exists to produce a coded error. Descriptors are copied rather than read, so an accessor is never invoked; building the message and copying the error are both wrapped, so a throwing `message` getter or a body that references itself now yields a plain redacted `Error` instead of a raw `TypeError`. The exact-value redaction was wired to the health poll alone. `startRelay` holds the preview URL from the moment the sandbox answers, so it now passes that credential to `relayStartError` and to `killSandbox` as well. Both nets keyed on the JWT's two literal dots, which any escaping proxy removes: the pattern accepts `%2E`, `%2e` and `.` as separators, and the exact-value comparison covers the same forms. The `unhandledRejection` listener in the handoff test is inert under bun — the runner claims the rejection first. The comment now says so instead of implying the assertion is the gate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Fixes the four findings from the GPT-5.6 Sol review of #6.
safeLoggerdid not contain a rejecting logger.Loggerdeclares
voidand TypeScript accepts anasyncmethod there, so a logshipper whose methods reject left an unhandled rejection — the runtime ends
the process for that, mid-handoff, before the relay sandbox is released. A
handler is now attached to the runtime return value inside the existing
try. Tests: a rejecting logger, a throwing property getter, and a fullhandoff (fake page, real relay process) driven with a rejecting logger.
pt_…. It survived bothpt_token%3D<jwt>and a bareinvalid preview token <jwt>.waitForHealthnow takes the exact token out of the URL it is polling and redacts that value
and its percent-encoded form by comparison; the pattern rules remain as the
second net, with a JWT-shaped rule added. The fixture is a JWT, and Sol's two
leak forms are tests that failed before the fix.
causekept the unredacted body and message. It is now aclone with the same prototype,
name,statusandcode, and withmessageand the parsedbodyredacted. The 429 test asserts oncause.messageandcause.body. README and CHANGELOG say thatcauseisthe original error with credentials redacted.
browser.isConnected()ran outsidecheckedPage's try. Abrowser proxy whose liveness accessor throws escaped as a plain
Errorinstead of
browser_unusable. It is read inside the try and branched onafter.
Also in here: the health poll no longer issues a final request with no budget
left, whose abort used to overwrite the proxy's own answer — the one useful
thing in
relay_not_ready's message — with "The operation timed out". TheCHANGELOG bullets Sol called overstated now claim exactly what is true.
Gates: lint, typecheck, 228 tests, build, dist-smoke (9 exports).
🤖 Generated with Claude Code