Skip to content

[o11y] Add internalError event outcome#6589

Merged
fhanau merged 1 commit intomainfrom
felix/041526-internal-error
Apr 22, 2026
Merged

[o11y] Add internalError event outcome#6589
fhanau merged 1 commit intomainfrom
felix/041526-internal-error

Conversation

@fhanau
Copy link
Copy Markdown
Contributor

@fhanau fhanau commented Apr 16, 2026

See internal discussion as for the rationale for this.
As noted in outcome.capnp, the authoritative version of EventOutcome is defined internally and needs to be updated first – this has already been done.
Note that internalError is not being used in workerd at present – we likely will use it in server.c++'s RequestObserverWithTracer::reportFailure(), but we should only start using it there once we've finalized how it will be used in the edge runtime.

Also see the internal PR.

@fhanau fhanau requested review from a team as code owners April 16, 2026 02:26
@fhanau fhanau requested a review from ascorbic April 16, 2026 02:26
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 16, 2026

The changes are clean, complete, and consistent. Let me verify one more thing - whether the ToJs switch in trace-stream.c++ is exhaustive (all enum values are covered):

The switch statement in trace-stream.c++:312-337 covers all 12 enum values (OK, CANCELED, EXCEPTION, KILL_SWITCH, DAEMON_DOWN, EXCEEDED_CPU, EXCEEDED_MEMORY, LOAD_SHED, RESPONSE_STREAM_DISCONNECTED, SCRIPT_NOT_FOUND, INTERNAL_ERROR, UNKNOWN), which matches the 12 values in outcome.capnp. The new INTERNAL_ERROR case is placed in correct alphabetical order (after SCRIPT_NOT_FOUND, before UNKNOWN), consistent with the ordering pattern of the switch.

The TypeScript type union in trace.d.ts is updated, and the generated snapshots match. The capnp ordinal @11 is correctly one past the previous maximum @10.

This is a minimal, well-structured addition of a new enum variant across all the right layers. No behavioral changes, no backward compatibility concerns (additive only), no security implications.

LGTM

github run

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 16, 2026

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing felix/041526-internal-error (d615a50) with main (5fdf8aa)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@fhanau fhanau force-pushed the felix/041526-internal-error branch from 7053e41 to fd0dcf8 Compare April 22, 2026 00:50
@fhanau fhanau force-pushed the felix/041526-internal-error branch 2 times, most recently from e6c3d59 to 48b0783 Compare April 22, 2026 15:39
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.48%. Comparing base (5fdf8aa) to head (d615a50).

Files with missing lines Patch % Lines
src/workerd/io/trace-stream.c++ 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6589   +/-   ##
=======================================
  Coverage   66.47%   66.48%           
=======================================
  Files         405      405           
  Lines      117773   117775    +2     
  Branches    19424    19425    +1     
=======================================
+ Hits        78295    78297    +2     
+ Misses      27900    27898    -2     
- Partials    11578    11580    +2     

☔ 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.

@fhanau fhanau force-pushed the felix/041526-internal-error branch from 48b0783 to d615a50 Compare April 22, 2026 16:19
@fhanau fhanau merged commit 46d402b into main Apr 22, 2026
22 of 23 checks passed
@fhanau fhanau deleted the felix/041526-internal-error branch April 22, 2026 16:59
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.

5 participants