Skip to content

fix(nextjs/instrumentation): warn about missing onRequestError handler #15488

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Feb 25, 2025

Conversation

a-hariti
Copy link
Collaborator

closes #15049

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

@a-hariti a-hariti requested review from lforst and chargome February 25, 2025 13:54
@@ -56,6 +56,7 @@ export function constructWebpackConfigFunction(

if (runtime !== 'client') {
warnAboutDeprecatedConfigFiles(projectDir, runtime);
warnAboutMissingonRequestErrorHandler(projectDir);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do this only for runtime === 'nodejs'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the runtime is either: client | edge | server

I used server for now

Copy link
Contributor

Choose a reason for hiding this comment

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

oh right. Yeah perfect

a-hariti and others added 3 commits February 25, 2025 14:23
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
and only warn about missing onRequestError when an instrumentation file
exists
@a-hariti a-hariti requested a review from lforst February 25, 2025 15:05
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
@a-hariti a-hariti merged commit 6e9682a into develop Feb 25, 2025
149 checks passed
@a-hariti a-hariti deleted the warn-missing-request-error-handler-2 branch February 25, 2025 15:38
@chargome
Copy link
Member

@a-hariti please use squash-merge next time 🙏

Comment on lines +447 to +452
const instrumentationPaths = [
['src', 'instrumentation.ts'],
['src', 'instrumentation.js'],
['instrumentation.ts'],
['instrumentation.js'],
];

Choose a reason for hiding this comment

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

Looks like this approach does not respect pageExtensions in Next.js config. I created #15652 to track this bug.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Next.js] Emit a warning when the user has not configured onRequestError on Next.js versions that support it
4 participants