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

[NextJs] process.env access causes undefined error when not polyfilling process.env #14193

Open
3 tasks done
arranf opened this issue Nov 6, 2024 · 5 comments
Open
3 tasks done
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK

Comments

@arranf
Copy link

arranf commented Nov 6, 2024

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

8.37.1

Framework Version

Next 15.0.2

Link to Sentry event

No response

Reproduction Example/SDK Setup

// sentry.client.config.js
import * as Sentry from "@sentry/nextjs";

Sentry.init({
  dsn: "DSN HERE",
  // Replay may only be enabled for the client-side
  integrations: [Sentry.replayIntegration()],

  // Set tracesSampleRate to 1.0 to capture 100%
  // of transactions for tracing.
  // We recommend adjusting this value in production
  tracesSampleRate: 1.0,

  // Capture Replay for 10% of all sessions,
  // plus for 100% of sessions with an error
  replaysSessionSampleRate: 0.1,
  replaysOnErrorSampleRate: 1.0,

  // Setting this option to true will print useful information to the console while you're setting up Sentry.
  debug: true,
});
// next.config.ts

async function config(phase, { defaultConfig }) : Promise<NextConfig> {
 return 
      {
      pageExtensions: ["js", "jsx", "tsx", "ts"],
      output: "standalone",
      experimental: { 
        // Reduces bundle size
        fallbackNodePolyfills: false,
      }
  }
}

module.exports = withSentryConfig(
  config,
  {
    // For all available options, see:
    // https://github.com/getsentry/sentry-webpack-plugin#options

    org: "ORG",
    project: "PROJECT",
    

    // Only print logs for uploading source maps in CI
    silent: !process.env.CI,

    // For all available options, see:
    // https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/

    // Upload a larger set of source maps for prettier stack traces (increases build time)
    widenClientFileUpload: true,

    // Automatically annotate React components to show their full name in breadcrumbs and session replay
    reactComponentAnnotation: {
      enabled: true,
    },

    // 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",

    // Hides source maps from generated client bundles
    hideSourceMaps: true,

    // Automatically tree-shake Sentry logger statements to reduce bundle size
    disableLogger: true,

    // Enables automatic instrumentation of Vercel Cron Monitors. (Does not yet work with App Router route handlers.)
    // See the following for more information:
    // https://docs.sentry.io/product/crons/
    // https://vercel.com/docs/cron-jobs
    automaticVercelMonitors: false,
  }
);

Steps to Reproduce

  1. Run next dev with the config above
  2. See an error TypeError: process.env is undefined
  3. Remove fallbackNodePolyfills
  4. Run next dev
  5. No error occurs

Expected Result

  1. You can run the Sentry NextJS SDK without polyfilling process.env

Actual Result

  1. node_modules/@sentry/nextjs/build/cjs/client/index.js's init function has multiple calls to process.env and assumes a polyfill.
@github-actions github-actions bot added the Package: nextjs Issues related to the Sentry Nextjs SDK label Nov 6, 2024
@arranf arranf changed the title nextjs client [NextJs] process.env access casues undefined error when not polyfilling process.env Nov 6, 2024
@AbhiPrasad
Copy link
Member

It's unfortunate that fallbackNodePolyfills also removes process.env, because at built-time webpack should just replace process.env.X with the according values (with EnvironmentPlugin). As such there should be no extra bundle size hit from any process.env logic.

I guess we should also support this use case given it makes sense to not use these APIs on the browser. Going to backlog, but PRs are welcome!

@AbhiPrasad
Copy link
Member

ref: https://nextjs.org/docs/pages/api-reference/next-config-js/env

Considering these are in the official Next.js docs, I'm even more hesitant in assuming any defaults where process.env does not exist. We'll be relying on this more as well as turbopack gets more mature: #14081

@arranf
Copy link
Author

arranf commented Nov 6, 2024

ref: https://nextjs.org/docs/pages/api-reference/next-config-js/env

Considering these are in the official Next.js docs, I'm even more hesitant in assuming any defaults where process.env does not exist. We'll be relying on this more as well as turbopack gets more mature: #14081

@AbhiPrasad I agree this is odd! In the project I discovered this on this option is set however when the polyfills option is turned off, these variables are not set for the client. I wonder if this is intended behaviour?

@lforst
Copy link
Member

lforst commented Nov 6, 2024

Just so that we're aligned... you mean that if you set fallbackNodePolyfills: false, the application crashes right? Cause in your report you're saying that we are crashing when the option is not set - which would really surprise me.

I looked a bit in the Next.js code base and I think what's happening here is that the Vercel folks may have forgotten that process.env is used with a lot of other features of the framework and it will simply break?

I honestly don't think we will work around this experimental option for now. I recommend turning it off. If it ends up becoming stable we will of course find a solution.

@arranf
Copy link
Author

arranf commented Nov 6, 2024

Just so that we're aligned... you mean that if you set fallbackNodePolyfills: false, the application crashes right? Cause in your report you're saying that we are crashing when the option is not set - which would really surprise me.

I looked a bit in the Next.js code base and I think what's happening here is that the Vercel folks may have forgotten that process.env is used with a lot of other features of the framework and it will simply break?

I honestly don't think we will work around this experimental option for now. I recommend turning it off. If it ends up becoming stable we will of course find a solution.

Sorry - yes, you're absolutely right. It fails when set to false. My fault for writing up a bug report at 2am after figuring this out!

That's fair enough - I do think you guys are right this should probably be an upstream change. I'll report this to Vercel.

@arranf arranf changed the title [NextJs] process.env access casues undefined error when not polyfilling process.env [NextJs] process.env access causes undefined error when not polyfilling process.env Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK
Projects
Status: No status
Development

No branches or pull requests

3 participants