Conversation
size-limit report 📦
|
30f6e44 to
a2593c6
Compare
Matches #24374: mechanism `auto.ai.flue` with `handled: false`, and the capture runs under the operation's own span so the issue lands on the right trace. Unlike Mastra, the rebuild from `errorInfo` stays. Mastra's `errorInfo` is `{ name, message }` with no stack, which is why it has to reach for the channel's real `Error`; Flue's carries the original stack, so there is nothing to gain from a second seam that might not fire on every path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mydea
changed the title
Matches #24374: mechanism `auto.ai.flue` with `handled: false`, and the capture runs under the operation's own span so the issue lands on the right trace. Unlike Mastra, the rebuild from `errorInfo` stays. Mastra's `errorInfo` is `{ name, message }` with no stack, which is why it has to reach for the channel's real `Error`; Flue's carries the original stack, so there is nothing to gain from a second seam that might not fire on every path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| const span = id ? getSentrySpanForMastraId(id) : undefined; | ||
| // `captureException` dedupes on the error instance, so the same error re-thrown through outer | ||
| // `executeWithContext` calls is captured only once. | ||
| const capture = (): string => captureException(error, { mechanism: { type: 'auto.ai.mastra', handled: false } }); |
There was a problem hiding this comment.
h: I think we should avoid capturing errors on the channel subscriber like we do with other subscriptions.
The tracing channel tells us that the promise/call rejected but doesn't guarantee it was handled by try/catch or .then/.catch or not.
The error subscriber is only good for setting span status, not capturing errors, which should still happen on global unhandled hooks.
There was a problem hiding this comment.
the problem for this specific thing here is, that mastra swallows these errors here - they generally do not bubble up 😬 so right now there is no way to capture these errors at all.
There was a problem hiding this comment.
if a user try-catches this inside of the tool call, it should not bubble here I suppose so I think it should be fine?
| function captureMastraError(error: unknown, params: unknown): void { | ||
| const id = isObjectLike(params) ? mastraSpanId(params.span) : undefined; | ||
| const span = id ? getSentrySpanForMastraId(id) : undefined; | ||
| // `captureException` dedupes on the error instance, so the same error re-thrown through outer |
There was a problem hiding this comment.
m: clanker raised this, Mastra seems to wrap errors with new MastraError so they would probably escape the dedup-by-identity logic we have.
I raised another point about not capturing at all which would make this pointless anyways.
There was a problem hiding this comment.
adjusted to handle this properly, good catch!
a2593c6 to
9563c57
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
When a Mastra tool or model operation throws, the exporter only reflected it on
the span (error status + stack-less `error.type` from Mastra's serialized
`errorInfo`); the error never became a Sentry issue.
Mastra runs each operation inside `executeWithContext({ span, fn })`, and when
`fn` rejects that channel's `error` event carries the real `Error` — stack and
all. The integration now subscribes to it and captures the error with
`captureException` (mechanism `auto.ai.mastra`, handled: false), associated with
the exporter's span for that operation so it lands on the right trace.
`captureException` dedupes on the error instance, so an error re-thrown through
outer operations is captured once. Capturing needs no async-context binding, so
it rides the attach-only path alongside the exporter registration.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9563c57 to
172d837
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 172d837. Configure here.
| captureMastraError(data.error, (data.arguments as unknown[] | undefined)?.[0]); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Channel errors captured too broadly
Medium Severity
The new subscriber calls captureException on every executeWithContext channel error event. That event only means the traced call rejected, not that the rejection is unhandled, so failures that still propagate to user try/catch or global hooks can be reported twice or marked handled: false after the app already handled them. Flagged because the review rules file requires this check for instrumentation that captures even when the error can still bubble.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 172d837. Configure here.
There was a problem hiding this comment.
I think this is not true, this generally doe snot bubble anywhere (at least not anywhere a user can get to it)
Mastra rethrows failures wrapped in `new MastraError({ cause })` — a different
object than the original — so `captureException`'s identity dedup wouldn't stop
the same failure being captured twice if it surfaces at both an inner and an
outer operation.
Track captured errors together with their `.cause` chain (bounded walk) in a
WeakSet, and skip an error if it, anything it wraps, or anything wrapping it was
already captured. `MastraBaseError` sets `this.cause` to the original, so the
chain reliably links the wrapper to the raw error.
Adds unit tests covering capture, wrap dedup, and distinct-error separation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const span = id ? getSentrySpanForMastraId(id) : undefined; | ||
| const capture = (): string => captureException(error, { mechanism: { type: 'auto.ai.mastra', handled: false } }); | ||
|
|
||
| // Attach to the operation's span so the issue lands on the right trace, when the span is still open. | ||
| if (span) { | ||
| withActiveSpan(span, capture); | ||
| } else { |
There was a problem hiding this comment.
Bug: A race condition can cause Mastra errors to be captured without being associated with their trace, as the span may be unregistered before the error handler runs.
Severity: MEDIUM
Suggested Fix
To prevent this race condition, ensure the span is accessible when the error is captured. This could be achieved by delaying the unregistration of the span until after the error handler has had a chance to process it, or by modifying the event data to include the span object directly, removing the need to look it up in the registry.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/server-utils/src/integrations/mastra.ts#L150-L156
Potential issue: When a Mastra operation fails, it fires both an `.end` event and an
`.error` event. A race condition exists where the span can be ended and unregistered by
the `.end` event handler before the `.error` event handler runs. When this happens, the
error handler's call to `getSentrySpanForMastraId()` returns `undefined` because the
span has already been removed from the registry. Consequently, the error is captured but
is not associated with its corresponding trace, losing important debugging context.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
i guess even if this happens it's fine, it will just lack this linking.
2 participants
Stacked on #24368.
When a Mastra tool or model operation throws, the exporter only reflected it on the span (error status + a stack-less
error.type, from Mastra's serializederrorInfo) — the error never became a Sentry issue.Mastra runs each operation's work inside
executeWithContext({ span, fn })(the same seam #24368 uses for nesting). Whenfnrejects, that channel'serrorevent carries the realError, stack and all. The integration now subscribes to it and captures the error withcaptureException(mechanismauto.ai.mastra,handled: false), insidewithActiveSpanof the exporter's span for that operation so the issue lands on the right trace.captureExceptiondedupes on the error instance, so an error re-thrown through outer operations is captured only once.Capturing needs no async-context binding, so it rides the attach-only path next to the exporter registration (not gated behind
waitForTracingChannelBinding).Root cause: the exporter receives
errorInfo({ name, message }), not the thrownError, so it can only set span status — there's no stack to build a real issue from. The actualErroris only observable at the throw site, whichexecuteWithContext's channel error surfaces.Verified in the node integration suite (Node 22, ESM + CJS): a thrown tool error is captured as an issue with
type: Error, the real message, a non-empty stack, and mechanismauto.ai.mastra/handled: false. Thenode-mastrae2e error test now also asserts the captured issue (not just the errored span), and the corresponding TODO is dropped.🤖 Generated with Claude Code