-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
TRPC Middleware Context not Working as Expected with Batch Requests #16262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Hi @17Amir17, thanks for filing this. This happens because there's no automatic scope forking going on for the middleware. We're looking into possible workarounds/solutions. |
Because there's no automatic scope forking, you'd have to fork your own scope using for example the Your trpc api would have to look something like this: ...
.query(async ({ input }) => {
return Sentry.withScope(async (scope) => {
scope.setTag("catRequestId", input.catRequestId);
await new Promise((resolve) => setTimeout(resolve, 1000));
if (input.shouldThrow) {
throw new Error("Error triggered by shouldThrow in getCats");
}
Sentry.captureMessage("getCats success");
return ["Whiskers", "Mittens", "Shadow"];
});
}),
}); Now, this works well for the second case, but it does not work for the first case because scope data is picked up when the error is captured, which happens in our trpc middleware. By then, the scope we fork ourselves is already gone and so is its data. If you want to solve the first case, you'd have to manually try/catch throwing code and capture the error manually using ...
.query(async ({ input }) => {
return Sentry.withScope(async (scope) => {
scope.setTag("catRequestId", input.catRequestId);
await new Promise((resolve) => setTimeout(resolve, 1000));
if (input.shouldThrow) {
try {
throw new Error("Error triggered by shouldThrow in getCats");
} catch (error) {
Sentry.captureException(error);
}
return;
}
Sentry.captureMessage("getCats success");
return ["Whiskers", "Mittens", "Shadow"];
});
}), Hope this helps. |
Writing tRPC handlers today and using top-level methods like `Sentry.setTag` isn't very intuitive as the isolations scope is not forked per procedure in our middleware. This PR changes the middleware to fork the isolation scope, while this is not 100% correct, as it breaks the one isolation scope per process/request model, it should be more intuitive and work better for most users. Resolves: #16262
A PR closing this issue has just been released 🚀This issue was referenced by PR #16296, which was included in the 9.20.0 release. |
@17Amir17 this should now work out of the box without having to fork the scope yourself. |
BEAST thanks <3 |
Is there an existing issue for this?
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/nextjs
SDK Version
9.17.0
Framework Version
React Next 15.3.1
Link to Sentry event
https://amir-lf.sentry.io/issues/41286609/?project=4509151724830800&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=issue-stream&stream_index=0
Reproduction Example/SDK Setup
Create a repo with minimal code that reproduces this https://github.com/17Amir17/SentryBug
Repo contains a simple nextjs app with trpc and sentry
One route called
getCats
this route accepts two params one which when true throws an exception and the the other contains an id which gets added as a tag - https://github.com/17Amir17/SentryBug/blob/main/src/pages/api/trpc/%5Btrpc%5D.tsSteps to Reproduce
There are two problem that I see, to reproduce the first
To reproduce the second
Expected Result
For both issues I'd expect to see two events and for the context to match the tag
Actual Result
For the first issue the tags do not match the context and there is only 1 event created
For the second issue the there are two events but the tag does not match the context
The text was updated successfully, but these errors were encountered: