Skip to content

Reclassify actor-class-as-entrypoint error as JSG TypeError#6400

Open
anna-cf wants to merge 2 commits intomainfrom
akapuscinska/fix-misclassified-error
Open

Reclassify actor-class-as-entrypoint error as JSG TypeError#6400
anna-cf wants to merge 2 commits intomainfrom
akapuscinska/fix-misclassified-error

Conversation

@anna-cf
Copy link
Copy Markdown
Contributor

@anna-cf anna-cf commented Mar 24, 2026

"worker is not an actor but class name was requested" was logged via LOG_ERROR_PERIODICALLY, causing it to hit Sentry as an internal error. This is a user/config mistake (a DO class requested through the non-actor dispatch path), so it should be a JSG TypeError surfaced to the caller instead.

@anna-cf anna-cf requested review from a team as code owners March 24, 2026 17:21
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 24, 2026

Good — the 500 response is still expected, confirming no user-visible behavioral change.

This is a small, well-scoped change:

  1. Replaces LOG_ERROR_PERIODICALLY (which hits Sentry as an internal error) with JSG_FAIL_REQUIRE(TypeError, ...) (which surfaces as a user error).
  2. JSG_FAIL_REQUIRE throws via KJ_FAIL_REQUIRE, so the subsequent KJ_FAIL_ASSERT on line 2372 is never reached for the actor-class case — only for the "no such named entrypoint" case.
  3. Test updated correctly: the three old log expectations are replaced with one matching the JSG TypeError format.
  4. No backward compatibility concern — the HTTP response is unchanged; this only changes internal error classification.
  5. No security concern — the error message doesn't leak new information.

LGTM

github run

@anna-cf anna-cf force-pushed the akapuscinska/fix-misclassified-error branch from 9b8ce63 to cdcc975 Compare March 27, 2026 00:00
@anna-cf anna-cf requested a review from a team as a code owner March 27, 2026 00:00
@anna-cf anna-cf requested a review from ascorbic March 27, 2026 00:00
@anna-cf anna-cf force-pushed the akapuscinska/fix-misclassified-error branch from cdcc975 to aad45ba Compare March 27, 2026 00:02
@anna-cf anna-cf removed the request for review from ascorbic March 27, 2026 00:03
anna-cf added 2 commits March 27, 2026 12:20
Pass isDynamicDispatch bool from newWorkerEntrypoint() into
getExportedHandler(), covering both the HTTP/connect path (via
WorkerEntrypoint) and the JS RPC path (via JsRpcSessionCustomEvent and
EntrypointJsRpcTarget).

The flag is passed as a parameter to CustomEvent::run() so every
implementation is required at the call site to forward it explicitly to
getExportedHandler(). All other CustomEvent::run() implementations
(trace, queue, hibernatable-web-socket, tail-stream) also pass it
through for consistency and forward-compatibility.
…atch

When isDynamicDispatch is true (entrypoint name supplied at request time),
two errors in getExportedHandler() are reclassified as JSG TypeErrors so
they surface to the caller rather than being logged as internal errors.
This will inform users about their likely misconfiguration:

- A DO class name requested via the non-actor dispatch path
- A non-existent entrypoint name

For static dispatch (isDynamicDispatch false) both cases retain their
existing LOG_ERROR_PERIODICALLY behaviour, since they can reflect genuine
bugs or infrastructure failures.
@anna-cf anna-cf force-pushed the akapuscinska/fix-misclassified-error branch from aad45ba to c6f39f9 Compare March 27, 2026 12:20
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 27, 2026

Merging this PR will improve performance by 12.32%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 69 untouched benchmarks
⏩ 129 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
Encode_ASCII_1024[TextEncoder][0/0/1024] 3.6 ms 3.2 ms +12.32%

Comparing akapuscinska/fix-misclassified-error (c6f39f9) with main (0d830b1)

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.

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