Repository navigation
withMonitor causes server to crash when callback throws an error #16749
Description
Is there an existing issue for this?
- I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- I have reviewed the documentation https://docs.sentry.io/
- I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/node
SDK Version
9.32.0
Framework Version
No response
Link to Sentry event
No response
Reproduction Example/SDK Setup
Repro repo here: https://github.com/zachkirsch/sentry-with-monitor-repro
Summary: If the callback passed to withMonitor throws an error, Sentry crashes the server.
Steps to Reproduce
import * as Sentry from "@sentry/node";
import express from "express";
const app = express();
app.get("/crash", async (_req, res) => {
try {
await Sentry.withMonitor("my monitor", async () => {
throw new Error("Oops!");
});
res.sendStatus(200);
} catch (error) {
console.log("Error caught");
res.sendStatus(500);
}
});
app.listen(8000, () => {
console.log("Server is running on port 8000");
console.log("To crash server: curl http://localhost:8000/crash");
});
Expected Result
In the code above, when curling the /crash endpoint, a 500 status should be returned to the user and the server should stay alive listening for more requests.
Actual Result
In the code above, when curling the /crash endpoint, the entire server crashes:
I believe the fix should be to delete this line:
The Promise.resolve(maybePromiseResult).then( is effectively run "in the background" and doesn't need to throw. maybePromiseResult (the result of the user's callback) is returned from withMonitor, and it's already the user's responsibility to catch it if it rejects.
Workarounds?
The only workaround is to not use withMonitor and instead rely on imperative methods like Sentry.captureCheckIn. When using withMonitor, you can't wrap your callback in a try/catch because then withMonitor would mark all crons as successful.
Activity
andreiborza commentedon Jun 30, 2025
Hi @zachkirsch, thanks for filing this.
We added rethrowing the error to fix errors in cron jobs not ending up in Sentry, see: #14163.
The problem is, that while it's indeed the user's responsibility to catch if it rejects, we (as in the SDK) need to be able to tell the user when they forgot this by capturing an error.
We need to come up with a solution that also ensures these errors don't go unnoticed.
zachkirsch commentedon Jun 30, 2025
@andreiborza what if you eliminate the background promise entirely? proof of concept here: http://github.com/getsentry/sentry-javascript/pull/16777
andreiborza commentedon Jul 1, 2025
@zachkirsch thanks, will take a look. I think in general you're all right, it's probably better to remove the throwing as a whole and live with the fact that no errors are captured in Sentry when thrown in functions wrapped with
withMonitor.Fix/nestjs cron issues (#16792)
andreiborza commentedon Jul 3, 2025
This should be fixed by #16792.
Thank you @zachkirsch for filing this and sending a PR to fix this tho!