Repository navigation
Keep host names out of error reports - #1938
Merged
Merged
Conversation
A deck opened from another machine is opened by that machine's name, and every frame of a page error carried it to the report (http://alices-macbook.local:4317/assets/index-abc.js:1:2345). Both scrubs now replace the host of every http(s) and ws(s) address: the page's own scripts become <deck>/assets/index-abc.js:1:2345, so a frame still says where in the bundle, and any other address keeps its scheme and path as https://<host>/... The path pass leaves the path after either placeholder alone, so scrubbing twice changes nothing.
Closed
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
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 changes
http://alices-macbook.local:4317/) used to send that name in every frame of a page error. Both scrubs (the server's, which runs on every error that leaves, and the page's, which runs on the crash text seeded into the feedback dialog) now replace the scheme-and-host of everyhttp(s)://andws(s)://address:<deck>/assets/index-abc.js:1:2345, so a frame still says where in the bundle and reads the same on every machine;https://<host>/v1/app/errors(a user in front of the host goes with it).<deck>or<host>alone, so scrubbing twice changes nothing.The API's second scrub applies the same rule already, so reports from older versions are stored without the host too.
Verification
npm run typecheckclean.npx vitest run --maxWorkers=3 --minWorkers=1): 890 files, 11676 tests passed.src/web/__tests__/error-report-origins.test.ts: 17 address shapes through both scrubs (Chrome, Firefox and Safari frames; hostname, loopback, IPv6 and LAN addresses; a dynamic-import failure; other hosts, a proxy with a user, a WebSocket, a blob URL, an already-scrubbed line), the pattern held equal between the two copies, and an error posted to/api/client-errorwith a stubbed fetch, asserting the report carries neither the host nor the port. Before the change all 31 behavioural cases failed with the host still in the output.error-report-paths.test.ts: the address lines that used to be kept verbatim moved to the new test, where they keep their path under<deck>; the drift test between the two copies stays green.