-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Next.js quick start guide for manual setup #12663
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Bundle ReportChanges will decrease total bundle size by 15 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
|
||
## Disable the Sentry SDK Debug Logger to Save Bundle Size | ||
- Instrument Next.js server actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why the first two options here don't provide links; is this content that's in development?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this is content that is currently on this page, but we want to move it to other pages. This will be done before this site goes live, and then we can link properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lforst wdyt about moving the server action stuff to another page (probably the new API page)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Much clearer 🤠
…ick-start/nextjs-manual
Thanks, @coolguyzone, for your feedback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! I love the Step 1, 2, 3, ... - it's much more a story than previously where it was a random assortment of options.
The stuff I brought up is pretty important I feel like. Happy to discuss.
--- | ||
|
||
If you can't (or prefer not to) run the [automatic setup](/platforms/javascript/guides/nextjs/#install), you can follow the instructions below to configure your application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the somewhat strong opinion we should keep this line. Doing the setup manually is a mistake 95% of the time and I would like to use every opportunity we get to direct people back to the automatic setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I added this info back at the top in an info alert
// Route browser requests to Sentry through a Next.js rewrite to circumvent ad-blockers. | ||
// This can increase your server load as well as your hosting bill. | ||
// Note: Check that the configured route will not match with your Next.js middleware, otherwise reporting of client-side errors will fail. | ||
tunnelRoute: "/monitoring", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a bad feeling about putting tunnelRoute here. It's a foot gun in combination with middlewares and the comment here is too untechnical. I'd think about moving it into its own step maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If tunneling is something we need to explain in more detail, it makes sense to deal with it in a separate section.
So I've added ## Step 5: Avoid Ad Blockers With Tunneling (Optional)
-- please check if this looks okay to you content-wise
<Alert type="info" title="Note"> | ||
You can also add Error components for the Pages Router, which you can learn | ||
more about here (LINK!). | ||
</Alert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pages router is still too ubiquitous for us to "hide away" that section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So let's either add an expandable or just inline the content again here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the section about Pages Router back in.
However, we don't have that in the installation wizard(?) or the wizard guide.
Does it make sense to add this as an optional step to the wizard guide?
(I also just realized that we didn't include the “global-error.jsx” in the list of things the wizard changes/creates in the user application. So I need to add this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely have it in the CLI wizard but not the getting started docs it seems like. We should probably add it. Thinking about it some more about it, I less and less like it that we include in the docs what the CLI wizard did or will do. The CLI wizard should be self-explanatory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why we put it in an expandable, so anybody who doesn't care about that can ignore it, and anybody else can have a look. I think it is a thoughtful service to users to offer this list
Do I need a custom next.js error page in my project before I run the wizard to have it add captureUnderscoreErrorException
? I didn't do that, and maybe that's why I can't find that in my project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the code the wizard should either create an _error
page automatically, or instruct you to add relevant code if you already the file.
@@ -307,386 +319,200 @@ import * as Sentry from "@sentry/nextjs"; | |||
export const onRequestError = Sentry.captureRequestError; | |||
``` | |||
|
|||
If you need additional logic within the `onRequestError` hook, call `captureRequestError` with all arguments passed to `onRequestError`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to keep this because we have no other location where this is documented and people will be clueless what to do in case they already have an onRequestError hook for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this also relevant for installation wizard users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially yes. However, my goal with my comment was to have this instruction ✨somewhere✨, at least so that Kapa or google can show it if people ask questions about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chargome will this go into the API page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inventarSarah we still don't know as the api page still needs more clarification – so let's keep it here for now
// Specify the organization and project to upload source maps to | ||
org: "___ORG_SLUG___", | ||
project: "___PROJECT_SLUG___", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would not remove, project is necessary, and org too with old auth tokens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this back in
|
||
Set the `disableLogger` option to `true` to strip the Sentry SDK debug logger out of your client bundles, saving bundle size. | ||
Please note that this will effectively override and disable the `debug` option in your `Sentry.init()` calls. | ||
<Expandable permalink={false} title="Did you experience any issues with this guide?"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know kinda off topic but this sounds way too self-conscious
<Expandable permalink={false} title="Did you experience any issues with this guide?"> | |
<Expandable permalink={false} title="Are you having problems setting up the SDK?"> |
1. Open the [**Issues**](https://sentry.io/orgredirect/organizations/:orgslug/issues) page and select an error from the issues list to view the full details and context of this error. For an interactive UI walkthrough, click [here](/product/sentry-basics/integrate-frontend/generate-first-error/#ui-walkthrough). | ||
2. Open the [**Traces**](https://sentry.io/orgredirect/organizations/:orgslug/traces) page and select a trace to reveal more information about each span, its duration, and any errors. For an interactive UI walkthrough, click [here](/product/sentry-basics/distributed-tracing/generate-first-error/#ui-walkthrough). | ||
3. Open the [**Replays**](https://sentry.io/orgredirect/organizations/:orgslug/replays) page and select an entry from the list to get a detailed view where you can replay the interaction and get more information to help you troubleshoot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this
<Alert level="warning" title="Important"> | ||
|
||
```javascript {tabTitle:ESM} {filename:next.config.mjs} | ||
export default withSentryConfig(nextConfig, { | ||
autoInstrumentMiddleware: false, | ||
}); | ||
``` | ||
|
||
```javascript {tabTitle:CJS} {filename:next.config.js} | ||
module.exports = withSentryConfig(nextConfig, { | ||
autoInstrumentMiddleware: false, | ||
}); | ||
``` | ||
Errors triggered from within your browser's developer tools (like the browser console) are sandboxed, so they will not trigger Sentry's error monitoring. | ||
|
||
### Instrumentation of Vercel Cron Jobs | ||
</Alert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably move this (devtools warning) into the "Verify > Issues" section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved it! 👍
- A Sentry [account](https://sentry.io/signup/) and [project](https://docs.sentry.io/product/projects/) | ||
- Your application up and running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: since we're gonna use this in a lot of places we could move it into an <Include ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 let me create a separate PR for this (it probably also makes sense to already use this Include
in the wizard guide)
<Alert type="info" title="Note"> | ||
You can also add Error components for the Pages Router, which you can learn | ||
more about here (LINK!). | ||
</Alert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So let's either add an expandable or just inline the content again here
|
||
<Alert title={<>Complications when using <code>tunnelRoute</code> with Next.js Middleware</>}> | ||
<Expandable level="warning" title="Are you developing with Turbopack?"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: maybe also move this one into an Include
, so we don't forget to update this once there are turbopack changes
|
||
## Disable the Sentry SDK Debug Logger to Save Bundle Size | ||
- Instrument Next.js server actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lforst wdyt about moving the server action stuff to another page (probably the new API page)?
…ick-start/nextjs-manual
Hey @lforst, @chargome @coolguyzone Note: We will only merge this after we've made sure that no content is lost (see #12487 ). This could mean that we need to temporarily add content back into this guide until we have created a suitable place for it. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good! So the only thing left is to clarify the api page and move (link) some content around right?
@@ -307,386 +319,200 @@ import * as Sentry from "@sentry/nextjs"; | |||
export const onRequestError = Sentry.captureRequestError; | |||
``` | |||
|
|||
If you need additional logic within the `onRequestError` hook, call `captureRequestError` with all arguments passed to `onRequestError`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inventarSarah we still don't know as the api page still needs more clarification – so let's keep it here for now
…ick-start/nextjs-manual
--- | ||
|
||
If you can't (or prefer not to) run the [automatic setup](/platforms/javascript/guides/nextjs/#install), you can follow the instructions below to configure your application. | ||
<Alert type="info"> | ||
For the fastest setup, we recommend using the [wizard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to also have this note include why one might choose manual setup? e.g. "Manual installation allows the greatest level of customization, for the fastest setup . . ."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, the user should take the same steps in this guide as the Installation Wizard. This means that the result is the same, and the user has no advantage or disadvantage if they prefer one guide to the other.
If this guide should have more content than the wizard guide at some point, then it makes sense to include the addition you suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!!
…ick-start/nextjs-manual
* commit first rough draft of page for a first review of structure * fix title typo * feedback updates * pr feedback * add tunneling alert * pr feedback * shorten and edit guide content * replace content with includes * formatting fixes * add back content that we can't move somewhere else atm
DESCRIBE YOUR PR
Rework the Next.js Manual Setup page into a quick start guide for manual setup.
Issue: #12486
This PR draft contains an early, WIP version of the quick start guide. Please focus your review on the structure and content of the page and ignore grammar, style, etc. for now, if possible :)
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
EXTRA RESOURCES