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

[arranf]

Is there an existing issue for this?

• [x] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• [x] I have reviewed the documentation https://docs.sentry.io/
• [x] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

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.

[AbhiPrasad]

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]

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: https://github.com/getsentry/sentry-javascript/pull/14081

[arranf]

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]

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]

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.