Skip to content
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

feat(nuxt): Set transaction name for server error #13292

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Aug 9, 2024

Setting the transaction name to display the API route where the error happened:
image

@s1gr1d s1gr1d requested review from Lms24, lforst and a team August 9, 2024 12:51
@@ -14,9 +14,17 @@ export default defineNitroPlugin(nitroApp => {
}
}

const currentScope = Sentry.getCurrentScope();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Is there a particular reason you're creating a binding for the current scope here instead of just calling getCurrentScope().setTransactionName() directly?

Comment on lines 19 to 23
const { method, path } = {
method: errorContext.event && errorContext.event._method ? errorContext.event._method : '',
path: errorContext.event && errorContext.event._path ? errorContext.event._path : 'unknown-path',
};
currentScope.setTransactionName(`${method} ${path}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: I recommend not setting the transaction name at all if the path is unknown, because we will give other sensible default mechanisms to activate and show in the product, ie. root span name, or stack trace.

@s1gr1d s1gr1d requested a review from lforst August 12, 2024 09:48
@s1gr1d s1gr1d self-assigned this Aug 12, 2024
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes!

@@ -1,4 +1,4 @@
import { captureException } from '@sentry/node';
import * as Sentry from '@sentry/node';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super-l/hint: It's totally fine to make a namespace (import * as) or named imports. No need to change it back, just fyi :)

@s1gr1d s1gr1d merged commit 0654dd0 into develop Aug 12, 2024
97 checks passed
@s1gr1d s1gr1d deleted the sig/nuxt-nitro-error-data branch August 12, 2024 15:03
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