Skip to content

strip sentry prefixes from tunneled exceptions#6690

Open
mikea wants to merge 1 commit intomainfrom
maizatskyi/2026-04-27-jsg-error
Open

strip sentry prefixes from tunneled exceptions#6690
mikea wants to merge 1 commit intomainfrom
maizatskyi/2026-04-27-jsg-error

Conversation

@mikea
Copy link
Copy Markdown
Contributor

@mikea mikea commented Apr 28, 2026

No description provided.

@mikea mikea requested review from danlapid and jasnell April 28, 2026 21:22
@mikea mikea requested review from a team as code owners April 28, 2026 21:22
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 28, 2026

This function strips "remote exception: " but not "SENTRY_" prefixes. If a broken-input-gate error were to get a SENTRY prefix somehow, this function would fail to detect it. But as I analyzed above, annotateBroken reconstructs the message without sentry prefixes, so this shouldn't happen in practice.

The code looks correct. The tests cover the main scenarios. Let me verify there are no issues I've missed with the overall logic and post my review.

The changes are clean and correct:

  1. stripSentryExceptionPrefix — correctly handles the two known SENTRY prefixes (SENTRY_DO , SENTRY_RT ) and safely returns the original string for unrecognized SENTRY_* patterns.

  2. The while-loop — correctly handles interleaved remote. and SENTRY_* prefixes in any order.

  3. Tests — cover SENTRY_DO, SENTRY_RT, and remote.SENTRY_DO patterns.

The code is safe, follows KJ conventions, and doesn't have backward compatibility concerns (it only affects internal error message parsing, not user-facing behavior).

LGTM

github run

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.42105% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.55%. Comparing base (d39911c) to head (4bc4c1b).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/workerd/jsg/util-test.c++ 33.33% 0 Missing and 10 partials ⚠️
src/workerd/jsg/exception.c++ 91.30% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6690      +/-   ##
==========================================
- Coverage   66.57%   66.55%   -0.03%     
==========================================
  Files         403      402       -1     
  Lines      115914   115924      +10     
  Branches    19412    19418       +6     
==========================================
- Hits        77170    77153      -17     
- Misses      27162    27189      +27     
  Partials    11582    11582              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mikea mikea enabled auto-merge (squash) April 28, 2026 23:10
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.

2 participants