getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.86k stars 1.55k forks source link

next build broken when wrapped with withSentryConfig #6782

Closed iscekic closed 1 year ago

iscekic commented 1 year ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/nextjs

SDK Version

7.30.0

Framework Version

nextjs 13.1.2

Link to Sentry event

N/A

SDK Setup

// next.config.js
// some properties omitted to keep it short

/** @type {import('next').NextConfig} */
const moduleExports = {
  reactStrictMode: true,

  swcMinify: false,

  experimental: {
    fallbackNodePolyfills: false,
  },

  eslint: {
    ignoreDuringBuilds: true,
  },
  typescript: {
    ignoreBuildErrors: true,
  },

  distDir: "build",

  poweredByHeader: false,

  sentry: {
    hideSourceMaps: true,
    widenClientFileUpload: true,
    tunnelRoute: "/api/sentry-tunnel",
  },
};

const sentryWebpackPluginOptions = {
  silent: true,
  release: "some-generated-string",
  errorHandler: (err, invokeErr, compilation) => {
    compilation.warnings.push("Sentry CLI Plugin: " + err.message);
  },
};

const wrapper =
  process.env.ANALYZE === "1"
    ? require("@next/bundle-analyzer")({
        enabled: true,
      })
    : (arg) => arg;

module.exports = wrapper(
  withSentryConfig(moduleExports, sentryWebpackPluginOptions)
);

Steps to Reproduce

  1. yarn next build

If withSentryConfig is removed, the build passes.

Expected Result

Build succeeds, next start runs successfully

Actual Result

NODE_ENV=production CI=1 yarn build

yarn run v1.22.19                                                                                                                                                                                                                                         
$ next build                                                                                                                                                                                                                                              
info  - Loaded env from /Users/igor/Projects/fireside-next-app/.env
warn  - You have enabled experimental feature (fallbackNodePolyfills) in next.config.js.
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.

info  - Skipping validation of types
info  - Skipping linting
[@sentry/nextjs] You are using edge functions or middleware. Please note that Sentry does not yet support error monitoring for these features.
✨  Done in 56.93s.
NODE_ENV=production CI=1 yarn next start

yarn run v1.22.19                                                                                                                                                                                                                                         
$ /Users/igor/Projects/fireside-next-app/node_modules/.bin/next start                                                                                                                                                                                     
ready - started server on 0.0.0.0:3000, url: http://localhost:3000
info  - Loaded env from /Users/igor/Projects/fireside-next-app/.env
warn  - You have enabled experimental feature (fallbackNodePolyfills) in next.config.js.
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.

/Users/igor/Projects/fireside-next-app/build/BUILD_ID
Error: Could not find a production build in the '/Users/igor/Projects/fireside-next-app/build' directory. Try building your app with 'next build' before starting the production server. https://nextjs.org/docs/messages/production-start-no-build-id
    at NextNodeServer.getBuildId (/Users/igor/Projects/fireside-next-app/node_modules/next/dist/server/next-server.js:173:23)
    at new Server (/Users/igor/Projects/fireside-next-app/node_modules/next/dist/server/base-server.js:58:29)
    at new NextNodeServer (/Users/igor/Projects/fireside-next-app/node_modules/next/dist/server/next-server.js:67:9)
    at NextServer.createServer (/Users/igor/Projects/fireside-next-app/node_modules/next/dist/server/next.js:143:16)
    at async /Users/igor/Projects/fireside-next-app/node_modules/next/dist/server/next.js:155:31
    at async NextServer.prepare (/Users/igor/Projects/fireside-next-app/node_modules/next/dist/server/next.js:130:24)
    at async /Users/igor/Projects/fireside-next-app/node_modules/next/dist/cli/next-start.js:118:9
error Command failed with exit code 1.                                                                                                                                                                                                                    
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lforst commented 1 year ago

Hi, withSentryConfig returns a function that you need to pass in the args next js passes to the default export. Please see:

iscekic commented 1 year ago

@lforst I'm not sure I understand what you're suggesting. I've changed the module.exports to:

module.exports = withSentryConfig(moduleExports, sentryWebpackPluginOptions);

I'm getting the same result.

The issue seems to be occuring with next 13.1.2 - an older version I'm using (13.0.6) works ok without any changes.

lforst commented 1 year ago

@lforst I'm not sure I understand what you're suggesting. I've changed the module.exports to:

module.exports = withSentryConfig(moduleExports, sentryWebpackPluginOptions);

I'm getting the same result.

The issue seems to be occuring with next 13.1.2 - an older version I'm using (13.0.6) works ok without any changes.

That should work. I did a test with next 13.1.2 and couldn't reproduce this issue. Can you try deleting your node_modules, reinstalling your dependencies, deleting your build folder, and rerunning next build && next start?

If that doesn't work, could you provide a small reproduction example so we can debug this further? Thanks!

iscekic commented 1 year ago

I deleted node_modules and build, ran next build and next start and I'm still getting the same result.

The output of the compilation seems to be there, other than the BUILD_ID file. Any hint on how I could better debug this?

lforst commented 1 year ago

The SDK doesn't do anything to the BUILD_ID file. I am still a bit skeptical that the SDK is responsible for causing this issue. Can you provide a minimal reproduction example we can clone?

iscekic commented 1 year ago

Looks like I can't get 7.31.0 to work at all due to https://github.com/getsentry/sentry-javascript/issues/6808

When 7.31.1 gets published I'll give it another shot and get back to you.

lforst commented 1 year ago

The release is out! Thank you! https://github.com/getsentry/sentry-javascript/releases/tag/7.31.1

baraeb92 commented 1 year ago

I have experienced the same, and it happens when using Sentry v.7.22.0 and above.

Likely due this change here: https://github.com/getsentry/sentry-javascript/pull/6291

This happens on the Next version 12 and above, but Output File Tracing on.

The error will be shown at Webpack and make the build fails. @lforst

lforst commented 1 year ago

@baraeb92 Can you share your next.js config? Thanks!

baraeb92 commented 1 year ago

@lforst

/**
 * @type {import('next').NextConfig}
 */
const isProd = process.env.NODE_ENV === 'production'
const runtimeCaching = require('./config/pwa/cache')
const withPWA = require('next-pwa')({
  dest: 'public',
  disable: !isProd,
  runtimeCaching,
})
const withBundleAnalyzer = require('@next/bundle-analyzer')({
  enabled: process.env.ANALYZE === 'true',
})
const { withSentryConfig } = require('@sentry/nextjs')
const SentryWebpackPluginOptions = {
  silent: true,
  setCommits: {
    auto: true,
  },
  validate: true,
  deploy: {
    env: process.env.BUILD_ENV,
    name: process.env.BUILD_ENV + ' release',
    url: process.env.NEXT_PUBLIC_SERVER_HOST,
  },
}
const nextConfig = {
  optimizeFonts: true,
  experimental: {
    adjustFontFallbacks: true,
    newNextLinkBehavior: true,
    nextScriptWorkers: true,
    legacyBrowsers: false,
    esmExternals: 'loose',
  },
  output: 'standalone',
  reactStrictMode: true,
  images: {
    minimumCacheTTL: 60,
    disableStaticImages: true,
    formats: ['image/avif', 'image/webp'],
    remotePatterns: [
      {
        protocol: 'https',
        hostname: '**.kitabisa.cc',
      },
      {
        protocol: 'https',
        hostname: '*.kitabisa.xyz',
      },
      {
        protocol: 'https',
        hostname: '*.kitabisa.com',
      },
      {
        protocol: 'https',
        hostname: 'firebase-kanvas.imgix.net',
      },
      {
        protocol: 'https',
        hostname: 'firebase-kanvas-staging.imgix.net',
      },
      {
        protocol: 'https',
        hostname: 'campaignership-*-ktbs.imgix.net',
      },
      {
        protocol: 'https',
        hostname: 'gudang-*.imgix.net',
      },
      {
        protocol: 'https',
        hostname: 'kitabisa-userupload-01.s3-ap-southeast-1.amazonaws.com',
      },
      {
        protocol: 'https',
        hostname: 'firebasestorage.googleapis.com',
      },
    ],
  },
  publicRuntimeConfig: {
    version: process.env.VERSION,
    configHost: process.env.ENV_NAME,
  },
  assetPrefix:
    isProd && process.env.CDN_ASSET_PREFIX.length > 0
      ? `${process.env.CDN_ASSET_PREFIX}/${process.env.VERSION}`
      : undefined,
  webpack: (config, { webpack }) => {
    config.module.rules.push({
      test: /\.(png|jpg|gif|webp|svg)$/i,
      type: 'asset/resource',
    })
    config.resolve.extensions = ['.js', '.jsx', '.ts', '.tsx']
    if (process.env.BUILD_ENV === 'production') {
      config.plugins.push(
        new webpack.DefinePlugin({
          __SENTRY_DEBUG__: false,
          __SENTRY_TRACING__: true,
        })
      )
    }
    return config
  },
  async rewrites() {
    return [
      {
        source: '/fbevents',
        destination: 'https://connect.facebook.net/en_US/fbevents.js',
      },
      {
        source: '/signals/config/:slug*',
        destination: 'https://connect.facebook.net/signals/config/:slug*',
      },
    ]
  },
  async redirects() {
    return [
      {
        source: '/explore',
        destination: '/explore/all',
        permanent: true,
      },
      {
        source: '/buat-campaign',
        destination: `${process.env.NEXT_PUBLIC_GALANG_DANA_URL}`,
        permanent: true,
      },
      {
        source: '/partners/:path*',
        destination: `${process.env.NEXT_PUBLIC_GALANG_DANA_URL}/partners/:path*`,
        permanent: true,
      },
    ]
  },
}
module.exports = () => {
  const plugins = [withPWA, withBundleAnalyzer]
  return plugins.reduce((acc, next) => next(acc), {
    ...(process.env.BUILD_ENV !== 'production'
      ? nextConfig
      : withSentryConfig(
          {
            ...nextConfig,
            sentry: {
              hideSourceMaps: true,
              autoInstrumentServerFunctions: true,
            },
          },
          SentryWebpackPluginOptions
        )),
  })
}

We only include Sentry on production build at the moment. This works on v7.21.0 but not on the v.7.22.0 above, is there any breaking changes that we should be aware of?

lforst commented 1 year ago

@iscekic @baraeb92 We'll probably release a new version on Monday that will resolve the issue for you. It's not really a fix but rather a revert of a change that broke a bunch of configurations that weren't dealing with the fact that withSentryConfig may return a function.

You can read up on it in this PR: https://github.com/getsentry/sentry-javascript/pull/6846

iscekic commented 1 year ago

Thanks @lforst

FelipeAfonso commented 1 year ago

Thanks, I'm having the same issue as well!

Sparragus commented 1 year ago

@iscekic @baraeb92 We'll probably release a new version on Monday that will resolve the issue for you. It's not really a fix but rather a revert of a change that broke a bunch of configurations that weren't dealing with the fact that withSentryConfig may return a function.

You can read up on it in this PR: #6846

lol I'm glad Sentry noticed this. I had to do the following to handle this:

module.exports = (phase, nextOptions) => {
    const extended = extend(nextConfig).withPlugins(plugins);
    const sentryExtendedNextConfig = sentry.extendNextConfig(extended(phase, nextOptions));

    // sentry.extendNextConfig can return either an config object or a function. Because we're
    // not sure which one it returns, we must check and handle each case.
    if (typeof sentryExtendedNextConfig === "function") {
        return sentryExtendedNextConfig(phase, nextOptions);
    } else {
        return sentryExtendedNextConfig;
    }
};
baraeb92 commented 1 year ago

Thanks @lforst

lforst commented 1 year ago

We released a new version of the SDK. Let me know if it resolves your issue!

kolorfilm commented 1 year ago

I have the same problem and tested already the latest version 7.32.1. With 7.31.1 I hadn't that problem. Also running next 13.1.2.

When try to build the next app with withSentryConfig(...) within the next.config.js I get this error: TypeError: withSentryConfig(...) is not a function

My next.config.js:

Bildschirm­foto 2023-01-24 um 12 02 17
lforst commented 1 year ago

@kolorfilm It's not necessary to call the return value of withSentryConfig anymore. Please take a look at the function's type definition. Note: We did not change the type definition across updates - it always returned either a function or an object. Please make sure you handle both cases.

kolorfilm commented 1 year ago

When I handle the return value like, for example, @Sparragus mentioned, it works. Thanks, also @lforst for the clarification.

lforst commented 1 year ago

@kolorfilm Glad you could get it resolved. This whole return value thing was a bit unfortunate (this PR description explains it a bit).

Right now the function actually returns a function if you pass in a function and it returns an object if you pass in an object, we should adjust the type to reflect that.

iscekic commented 1 year ago

@lforst This is still broken for me, but I did manage to find the root cause, which is swcMinify: false. Removing that line makes the build work as expected.

While I did get it to work, I prefer not using swc because it results in a larger bundle and can be buggy across nextjs upgrades.

lforst commented 1 year ago

@iscekic Interesting. We have integration tests where we're not using swc and everything compiles fine. Can you provide a reproduction example? Thanks!

mgoodfellow commented 1 year ago

We have been suffering this issue as well - we found the breakage happened after NextJS 13.1.1 - i.e. 13.1.2 and onwards cause this issue.

The failure mode is really odd and took me a while to spot - here is the output from a failing yarn build

image

Notice that it ends prematurely during source map upload and the builder comes back as DONE. It still has a lot more source maps to upload (all the client ones etc.)

Same build, but with 13.1.1 instead of 13.1.2:

image

The build continues after the source map upload.

We confirmed (for us) that turning off source map uploads resolved the build issue for us:

        disableServerWebpackPlugin: true,
        disableClientWebpackPlugin: true,

Obviously this isn't ideal as although we can get a build through, we have no source map visibility in our releases currently.

However, with the new information from @iscekic about swcMinify: false breaking it I can confirm that removing it also resolves it for us. However, for similar reasons we don't want to use the swcMinifier.

Quick aside:

The SDK doesn't do anything to the BUILD_ID file. I am still a bit skeptical that the SDK is responsible for causing this issue. Can you provide a minimal reproduction example we can clone?

This is the really strange failure mode of this bug - yarn build thinks it finished (no errors), but actually it never collected the page data and finished the build and emitted the compiled app - it dies during source map upload. That's why when you then run yarn start it gives the BUILD_ID error - it hasn't actually been built - "Error: Could not find a production build"

Max29a commented 1 year ago

I am also having this issue. I also do not want to use the swcMinifier. next: ^13.1.1 sentry/nextjs: ^7.33.0

lforst commented 1 year ago

Okay so just testing with a test app of my own, Next.js 13 doesn't seem to like swcMinify: false at all, even with Sentry completely disabled. I am still a bit confused as to what's going on. We have contacted the Vercel folks on a separate channel - let's see if we can figure this out.

lforst commented 1 year ago

I could pinpoint this down to having swcMinify: false and instructing webpack to emit sourcemaps with either devtool: 'source-map' or devtool: 'hidden-source-map'. withSentryConfig will set the devtool option so source maps can be uploaded.

This seems to be primarily an issue with Next.js so I will raise an upstream issue that hopefully gets resolved soon. In the meanwhile, I recommend using swc. From my personal experience, it has gotten pretty stable and the output isn't too large either.

Edit: Here's the issue: https://github.com/vercel/next.js/issues/45276

lforst commented 1 year ago

Okay it seems like this got upstream-fixed by https://github.com/vercel/next.js/pull/45423

javierojgs commented 1 year ago

Would be good to update the nextjs documentation here: https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#extend-nextjs-configuration

lforst commented 1 year ago

@javierojgs What do you recommend we update it with?

github-actions[bot] commented 1 year ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

peterb0yd commented 1 year ago

Seeing this issue on Next.js v12.3.4

Any idea how to fix?

lforst commented 1 year ago

@peterb0yd please open a new issue with details of the errors you're seeing and your setup. Thanks!