-
-
Notifications
You must be signed in to change notification settings - Fork 9
Module parse failed: Unexpected character '�' (1:0) #170
Comments
@haydenbleasel you need to define a loader for .node files. This seems like it uses webpack and not turbopack so https://webpack.js.org/loaders/node-loader/ should be the correct way to go. A quick search reveals this comment from nextjs team which explains how to set this up vercel/next.js#34040 (reply in thread) |
@JonasBa
|
We're experiencing this problem as well.
Here are our build logs when we try to use Next.js with App Router
|
I'm looking into this issue today, from what I can tell, the message you see at build time |
@doinki the warning is unfortunate, but I don't think we can get rid of this without breaking support for folks who don't use bundlers (which is a lot of nodejs application). The reason you see the warning is because in cases where we do not find the prebuild binary for the target arch, we default to the version which was built from source - this way users on older/newer versions of nodejs or some "non standard" arch can still use the package. The problem is that we do not know all the possible permutations of those target environments, which means they are not statically analyzable by bundlers and hence cannot be hardcoded in the list. Our binary loading logic handled this by having a default case in the switch platform/arch/node version (see cpu_profiler.ts), but here is a contrived example...
So as you see, if we remove the require expression, we end up breaking support for everyone not in the hardcoded list, which we don't want to do. This is unfortunate and I agree, annoying, but I would rather deal with a warning than breaking the package. That said, I am going to try and see if I can use the contextReplacementPlugin from webpack to silence this warning. |
same problem happening with the following packages
|
@karamgaby can you share the error you are seeing? There are different errors in this issue - if you are only seeing a warning If the error you are seeing is
then you need to ensure your build system knows how to handle .node files. You can either configure a loader or mark the package as external (see our guide on bundling) |
@JonasBa mind providing an example with how this would work with vercel ncc? I cannot get this to work with webpack and nextjs as an external |
This worked for me:
Not really, |
@mmahalwy we have a section on bundlers in our readme, you can see how to configure it there https://github.com/getsentry/profiling-node#bundling @Maxim-Mazurok what is the error that you see at build time? |
So I did And now I get this:
I've tried using latest stable next, and here's the next config:
my next setup is pretty vanilla, using app router. sentry configs also pretty much as wizard created them:
etc, hope it helps. I decided to defer node profiling, it's not very important for me at this stage of the project. |
@Maxim-Mazurok, that is very weird. We do not use the critters package so it's unclear to me why this happens. The error I'm seeing seems like a nextj react prerender error unrelated to @sentry/profiling-node
I'm going to try and setup a nextjs app and see if I can reproduce |
I also use MUI components and most of my components are |
Related to critters: vercel/next.js#34763 |
Hi I wanted to reopen this thread. I'm trying to setup profiling for my next.js application. I modified
and have the following sentry versions defined:
. This causes my build to fail with the following error:
I saw this related thread and added the following line to my
and then installed
which wasn't very illustrative. I was wondering if there were any ideas on how to handle this. |
@nhewage1997 I have the same problem. Haven't you fixed it yet? Versions:
|
@nhewage1997 you shouldn't need to install critters for this to work. Do you mind uninstalling it and telling me if it still fails? @tua-Mascot can you please share a relevant stack trace and confirm that you marked @sentry/profiling-node as an external module? |
I have the same issue when trying to integrate profiling in a nestjs project in mono-repo mode. Adding the package as external does not work. I removed profiling for now until the issue is fixed. |
@JonasBa Sure Stack trace
And I added this to next.config.js
I can provide some additional information:
|
@JonasBa Sorry to bother you, but more than a week has passed. Are there any updates/news? |
@tua-Mascot mind creating a reproducible case for this? I think that adding your component to external packages didn't work as the build system is still trying to resolve that file |
I have the same issue, when marking it as external I'm missing the "critters" package. |
Here's a reproducible case for this issue. Checking out this commit produces the following error. Ref: anonrig/yagiz.co@ef3c8af
|
Adding |
Getting this error as well on a new project with Sentry setup. I've tried doing ... Key relevant packages
Happy to try and provide more info if that will help, otherwise disabling profiling for the time being Simplest repro:
|
@JesseKoldewijn that would be a very good idea, thank you for proposing that. P.S: I'm happy to open the PR, I didn't think NextJS would allow something like that and was under the impression they'd want developers to manage this on their own |
Im opening an issue on Next.js's side already and added package to this list in the 13.x releases as well. so I prob got the PR up in a couple of minutes 👍 |
Issue created: vercel/next.js#60853 |
PR is open, lets see when it will be merged. Also noticed that this wasn't the first sentry package in there funny enough 😉 |
… breakage (#60855) Issue: #60853 ### What? Added `@sentry/profiling-node` to the server-external-package object so that sentry users who want to use this package don't have to manually add this package in the experimental nextjs config entry. ### Why? Package bundling will fail as shown in the following issue if not added to this exclude list. getsentry/profiling-node#170 --- Fixes #60853
Just woke up and saw that the fix was merged into the canary branch of Next.js🤙 |
Incase anybody wants to bypass setting the experimental config entry in Next.js already and is fine with using the canary channel of Next.js you can already use the fix I pushed through the PR by upgrading to Next.js release v14.1.1-canary.1 |
Part of what is frustrating with this issue is that many folks run this through platforms that largely abstract build configurations while also building and executing your code for different environments (example is server vs edge) and depending on their versions, they might even use different underlying build tools. The result of this is that many folks are not aware of how their code is built (rightfully so as tools should just work) or how their code is executed at runtime, leaving library maintainers largely in a bit of a tough spot to debug. When you see an error like Marking packages as external fixes this as it instructs the bundler to skip such files and instead require them at runtime, and while this does work, it leaves you without the realizing the gains that bundles code has on your script cold start performance (something highly relevant to something like profiling where we want to keep the runtime impact minimal) In other words, by requiring packages marked as external, the module loader needs to perform additional disk i/o to be able to evaluate the module and execute it, which impacts cold start performance. AWS has a good article on optimizing cold start performance in serverless that I recommend reading. All this to say that when you do encounter issues like this, you should check with your platform provider or your bundler configuration if it is possible to bundle packages containing binary files as it will both make your code faster to run as well as save you some infra costs. |
I forgot to add that in order to mitigate the impact on your cold start performance when |
@JonasBa I made a fresh repository with next.config.js and sentry configs are close to the settings in my project.
But if you delete the "use client", then everything works. Link to the repository - https://github.com/tua-Mascot/sentry-profiling-node-test |
In that case it crashes because the "use client" flag indicates that the component also has to run in de browser. Due to this flag the component wont be using the external package config or the Next.js internal packages list. Its kinda the same problem as when you try to run native node.js modules in the browser. |
@JesseKoldewijn |
If you only use the node-profiler within a server action, you can. Otherwise, you can't. Since a client component runs as mentioned before both on the server and client👍 |
Is there a going to be a fix in the future for support for the 'use client' directive for the nextjs app router in the future for profiling? |
The node profiler is not intended to run on the client since the browser doesn't run node. So as long as you exclude it from running on the client it might run fine in client components. |
@JesseKoldewijn Unfortunately, not work for me. Here is the verison I used |
Are you using the node profiler in a client component by chance? |
@JesseKoldewijn No, I use the default setting in my sentry.server.config file only. As the code below And I found there are confilts during install, because @sentry/next need a peerDependence for nextjs highest 14.0. |
Just to add to this, I also have the same issue. I tried fixing it by following the various recommendations you gave :
I also use MUI components, so I got |
As mentioned before, the package does work in Next.js 14.1.1-canary.1 and later. But without a reproduction repo and only the server config file I can't really make an educated guess on why it doesn't work in your project. |
Could you provide me with a reproduction repo by chance? Since I don't know where you import the profiling-node package right now. |
To anybody with this issue past next release 14.1.1-canary.1 please check out my repo below which is a working example of the fix within nextjs I've implemented. |
I had similar issues but running Node 18, Express.js, and Webpack. I was able to get passed the
But now I'm experiencing a new error message that I have not figure out how to resolve during webpack build. Does any have any suggestions?
|
I've ran into a similar issue. Tried to add
Which works as long as I don't have a
Wondering if there is a way to only enable the Profiling in the |
Are you importing the @sentry/profiling-node package inside a component which is marked with "use client"? Because this will not work due to the component also runs in the browser. |
Thank you @JesseKoldewijn . Sorry for my late response as well. We don't import @sentry/profiling-node package in any specific component. This is how we configured Sentry Profiling. We only has the import in the config files. I guess this enables the profiling for all components? // sentry.server.config.ts
import * as Sentry from '@sentry/nextjs';
import { ProfilingIntegration } from '@sentry/profiling-node';
Sentry.init({
profilesSampleRate: 1,
integrations: [new ProfilingIntegration()],
}); // sentry.client.config.ts
import * as Sentry from '@sentry/nextjs';
Sentry.init({
profilesSampleRate: 1,
integrations: [
new Sentry.BrowserProfilingIntegration(),
],
}); // next.config.mjs
import { withSentryConfig } from '@sentry/nextjs';
export default withSentryConfig({
headers: async () => {
return [
{
source: "/(.*)",
headers: [
{
key: "Document-Policy",
value: "js-profiling"
}
]
}
]
},
}); |
Is there an existing issue for this?
How do you use Sentry?
Sentry Saas (sentry.io)
SDK Version
1.0.7
Link to Sentry event
No response
What environment is your node script running in?
"@sentry/nextjs": "^7.57.0"
How is your code deployed and bundled?
Standard Next.js build process running on Vercel. Here's the abstracted
next.config.js
:Steps to Reproduce
Following the installation instructions in the Sentry web UI, updated my
sentry.server.config.js
to the following:Deploy to Vercel which runs
npm build
.Potential follow-on issue from #164
Expected Result
The build should pass.
Actual Result
The text was updated successfully, but these errors were encountered: