Repository navigation
Conversation
…ep callback `instrumentWorkflowWithSentry` only captured errors thrown inside a `step.do` callback. An error thrown in `run()` itself, a step timeout, a `step.waitForEvent` timeout or an engine-side step failure failed the instance without a Sentry event. `run()` now captures the error that fails it, as unhandled. The engine rejects `step.do` with a copy of the error the callback threw, so the copy is marked as captured when the callback already reported it. This keeps a failed step at one event. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Contributor
size-limit report 📦Compared: 1350e03 (PR test merge using develop at 6bee2f2) Against: 6bee2f2 (develop) · base run #65818 ✅ Baseline matches: this diff contains only this PR's changes.
|
javascript-sdk-gitflow
Bot
requested review from
HazAT,
andreiborza and
isaacs
November 12, 2026 11:11
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 2 not successful, 33 successful. See the run.
Successful checks
|
mydea
reviewed
Nov 12, 2026
| return stepResult.catch(error => { | ||
| // The engine rejects with a copy of the error the callback threw, so the copy has to be marked | ||
| // as captured too. Otherwise `run` would report the same failure a second time. | ||
| if (capturedByCallback) { |
Member
There was a problem hiding this comment.
do we need this capturedByCallback thing if we pass it through checkOrSetAlreadyCaught anyhow...?
Member
There was a problem hiding this comment.
ah wait, I think I am confused by the code. we ignore the return value here and just use this to mark this as captured, right? maybe worth it to adjust the comment a bit, e.g. "we use checkOrSetAlreadyCaught to ensure the error is marked as already sent" or something like this
This branch has not been deployed
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.
closes #26286
closes JS-4097
Description
This fixes
instrumentWorkflowWithSentryso that each error that failsrun()is sent to Sentry: an error thrown inrun()itself, astep.doorstep.waitForEventtimeout, and a step failure in the engine. Before, only errors thrown inside astep.docallback were captured.Mechanism decisions
Errors captured in
run()havehandled: false, because they fail the instance. Errors captured in the step callback keephandled: true. The alternative ishandled: falsefor a failed final step attempt too.Event count decisions
A failed step that fails the instance gives one event. But when
run()catches a step error and throws a new error, Sentry now gets two events: the step error and the new error. They are two different errors, and there is a test for this. The alternative is to capture only inrun(), but then a step error thatrun()catches and survives gives no event.Sentry events / traces
Error thrown in
run()before a step: https://sentry-sdks.sentry.io/issues/7844396928/events/7e1ad493e0ac44f8b23f2f6e534cd693/step.dotimeout: https://sentry-sdks.sentry.io/issues/7844396929/events/cdfb9eef82cb41d6a5b2072d6aa7c62f/Error thrown inside a step callback (one event): https://sentry-sdks.sentry.io/issues/7844396894/events/8cc4126912f640ee8d9be04d243b6e0b/
Trace with two errors,
run()replaces a failed step error: https://sentry-sdks.sentry.io/explore/traces/trace/c1b52ecbf72646e38638479f80b9740d/?project=4510555608449024