Skip to content

Skip logging script startup exceptions for dynamic workers#6332

Merged
ketanhwr merged 2 commits intomainfrom
ketan/dyworker-startup
Mar 16, 2026
Merged

Skip logging script startup exceptions for dynamic workers#6332
ketanhwr merged 2 commits intomainfrom
ketan/dyworker-startup

Conversation

@ketanhwr
Copy link
Member

No description provided.

@ketanhwr ketanhwr requested review from a team as code owners March 16, 2026 16:26
Co-authored-by: ketanhwr <ketanhwr@users.noreply.github.com>
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 16, 2026

The change is clear and straightforward. Let me analyze it:

Before the change: KJ_LOG(ERROR, ...) was called unconditionally for both dynamic and non-dynamic workers, then the code branched on isDynamicWorker to either rethrow or KJ_FAIL_REQUIRE.

After the change: KJ_LOG(ERROR, ...) is only called in the else (non-dynamic worker) branch, right before KJ_FAIL_REQUIRE. Dynamic workers skip the KJ_LOG entirely and just rethrow.

This makes sense — for dynamic workers, the exception is rethrown to the caller as a JS Error, so the caller can handle it. Logging it as an ERROR in the server logs would be noisy for what is expected user-facing behavior. The span logging (span.addLog(...)) still happens for both paths, preserving observability through tracing.

This is a clean, minimal, correct change. No backward compatibility concerns (this is internal logging behavior, not user-facing API). No safety issues. The span-based tracing is still unconditional. The logic is sound.

LGTM

github run

@ask-bonk ask-bonk bot requested a review from a team as a code owner March 16, 2026 16:27
@ketanhwr ketanhwr enabled auto-merge March 16, 2026 16:50
@ketanhwr ketanhwr merged commit bc0bc2a into main Mar 16, 2026
26 of 27 checks passed
@ketanhwr ketanhwr deleted the ketan/dyworker-startup branch March 16, 2026 18:23
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.

3 participants