[timfish]

Fixes #4829

This was caused by two issues:

• The code does not cater for the Windows binary name ending in .exe
• Currently, the binary doesn't actually end up where it should on Windows! On Windows, the binary is copied to an incorrect path sentry-cli#1368

Since the CLI getPath() function returns the correct binary location now, and after the above issue is fixed, this PR seems like a sensible route.

[timfish]

Why doesn't @sentry/cli also download the binary if it's missing when called via JavaScript?

All the access seems to go through the async execute function. Why not also check the presence of the binary there and download it if it's missing?

[timfish]

The failures are rather strange:
Unable to resolve path to module '@sentry/nextjs'

Maybe caused by using a non-direct dependency (ie. @sentry/cli).

Perhaps SentryWebpackPlugin should have a static function on it that gets the binary path from the CLI?

This would help with the fact that the @sentry/cli types only seem to work correctly with esModuleInterop enabled.

[lforst]

Why doesn't @sentry/cli also download the binary if it's missing when called via JavaScript?

All the access seems to go through the async execute function. Why not also check the presence of the binary there and download it if it's missing?

Sadly don't have the answer to those questions. @kamilogorek have you ever thought about doing this?

[lforst]

As for the errors: You may need to add @sentry/cli to the rollup config's external field, as it is not listed in the package.json's dependencies. I think rollup attempts to bundle/vendor the @sentry/cli and stuff gets weird.

I think it would go here:

sentry-javascript/packages/nextjs/rollup.npm.config.js

Line 12 in 31bf467

packageSpecificConfig: { external: ['next/router'] },

[timfish]

You may need to add @sentry/cli to the rollup config's external field,

Thanks @lforst, that appears to have fixed it!

[lforst]

The one thing we need to check here is if @vercel/nft will still not include the binary. We had a bunch of problems where users reported they couldn't deploy their projects to lambdas/vercel because the Sentry CLI binary was included in the build and it has something like 15MB pushing them over the 50MB limit.

It was mainly this issue: #3865

[timfish]

The CLI uses this code (and relevant PR) for the binary path, there will be no path to auto resolve:

/**
 * Absolute path to the sentry-cli binary (platform dependent).
 * @type {string}
 */
let binaryPath = eval(
  "require('path').resolve(__dirname, require('os').platform() === 'win32' ? '../sentry-cli.exe' : '../sentry-cli')"
);

Further context:

• Abhi added eval to fix nft
• Katie then replaced the eval(Katie reports that rollup complains about eval)
• There is an open issue about yarn pnp

There are alternatives to eval since @vercel/nft is fooled by basic obfuscation like:

function getPath(): string {
  const parts = [];
  parts.push(__dirname);
  parts.push('..');
  parts.push(`sentry-cli${process.platform === 'win32' ? '.exe' : ''}`);
  return path.resolve(...parts);
}

const binaryPath = getPath();

[timfish]

To avoid Rollup warnings about eval my preference to this PR is to:

• Merge fix: Replace usage of eval to obfuscate binary path from bundlers sentry-cli#1374
• New release v1 branch of @sentry/cli
• Merge feat: Add a means to detect if the CLI binary exists sentry-webpack-plugin#402
• Update sentry-webpack-plugin to use new version release of CLI
• New release of sentry-webpack-plugin

and then simply use SentryWebpackPlugin.cliBinaryExists() to detect if the binary exists.

[lforst]

I like that plan! Lemme go through it all. @lobsterkatie do you have any thoughts on this going forward?

[lobsterkatie]

To avoid Rollup warnings about eval my preference to this PR is to:

• Merge feat: Replace usage of eval to obfuscate binary path from bundlers sentry-cli#1374
• New release v1 branch of @sentry/cli
• Merge feat: Add a means to detect if the CLI binary exists sentry-webpack-plugin#402
• Update sentry-webpack-plugin to use new version release of CLI
• New release of sentry-webpack-plugin

and then simply use SentryWebpackPlugin.cliBinaryExists() to detect if the binary exists.

I think we need to add:

• Merge a copy of fix: Replace usage of eval to obfuscate binary path from bundlers sentry-cli#1374 into master as well
• Update JS SDK to use new webpack plugin version
• Release new version of JS SDK

But otherwise that sounds like a totally reasonable plan to me.

[timfish]

This PR is superseded by #6096 which uses the route described in the above comments.