v8 Node + ESM + esbuild error

[jd-carroll]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• 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/aws-serverless

SDK Version

8.0.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

For completeness I thought I would link this here too: open-telemetry/opentelemetry-js#4691

When bundling @sentry/...@^8 in a "full ESM" mode, you will get (potentially) hundreds of errors along the lines of:

WARN  ▲ [WARNING] Constructing "ImportInTheMiddle" will crash at run-time because it's an import namespace object, not a constructor [call-import-namespace]
   ../../node_modules/@prisma/instrumentation/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:273:30:
     273 │             var esmHook = new ImportInTheMiddle([
         ╵                               ~~~~~~~~~~~~~~~~~
 Consider changing "ImportInTheMiddle" to a default import instead:
   ../../node_modules/@prisma/instrumentation/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:55:7:
     55 │ import * as ImportInTheMiddle from 'import-in-the-middle';
        │        ~~~~~~~~~~~~~~~~~~~~~~
        ╵        ImportInTheMiddle

I have not taken the time to identify how / when this error could be encountered, but can confirm that doing the following in a module environment will throw an error:

import * as ImportInTheMiddle from 'import-in-the-middle';
var esmHook = new ImportInTheMiddle( ... );

The instantiation would need to look like:

var esmHook = new ImportInTheMiddle.default.default( ... );

Would be nice to add some support for either of the following issues:

• "ImportInTheMiddle" will crash at run-time open-telemetry/opentelemetry-js#4691
• fix: use require for import in the middle open-telemetry/opentelemetry-js#4546

If for no other reason than eliminating unnecessary warnings during build.

Expected Result

NA

Actual Result

NA

[mydea]

Hello,

this is hopefully fixed by #12017!

[lforst]

I think this will be an issue for all situations where esbuild is involved. @jd-carroll already you already did the right thing reporting this upstream! As soon as that fix lands in the opentelemetry packages we will bump the dep asap.

[jd-carroll]

@mydea @lforst This is 100% causing my lambdas to crash and makes using Sentry@v8 not possible for ESM.

Again, at runtime, the lambdas fail with:

That stacktrace corresponds to this code:

Could someone from @sentry help identify what the path forward for ESM would be?

There seem to be a number of pertinent issues:

• coordinate first pass at ESM support for all instrumentations open-telemetry/opentelemetry-js-contrib#1942
• [instrumentation] support ESM instrumentation for Node.js v18.19, v20, v21 open-telemetry/opentelemetry-js#4553
• Wrong import for "import-in-the-middle" with ESM open-telemetry/opentelemetry-js#4717
• wrong import for "import-in-the-middle" open-telemetry/opentelemetry-js#3954

[mydea]

Hey,

this is most likely fixed by open-telemetry/opentelemetry-js#4546 - we'll try to get this merged & released!

[AbhiPrasad]

@jd-carroll could you share the esbuild config you use?

[jd-carroll]

I’ll work on pulling together a small repro, but if you’re working on it yourself, make sure the generated file uses the mjs extension. (This is the recommended approach for ESM + lambda)

…

On Mon, May 27, 2024 at 9:48 AM Abhijeet Prasad ***@***.***> wrote: @jd-carroll <https://github.com/jd-carroll> could you share the esbuild config you use? — Reply to this email directly, view it on GitHub <#12009 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOMNKHEOBALH4CPPS73KETZENPXRAVCNFSM6AAAAABHU5CNQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZTHAZDIMJZGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jd-carroll]

Here is the raw config:

    // LambdaBundlerConfig
    const LambdaBundlerConfig: BuildOptions = {
      entryPoints: [LambdaEntryFile], // some/file.ts
      outfile: LambdaFile, // options.format === 'cjs' ? 'dist/lambdas/lambda.js' : 'dist/lambdas/lambda.mjs'
      bundle: true,
      sourcemap: 'linked',
      splitting: false,
      platform: 'node',
      mainFields: options.format === 'cjs' ? ['main'] : ['module', 'main'], // almost always ['module', 'main']
      external: options.external || [],
      target: options.target ?? (options.format === 'cjs' ? 'es5' : 'esnext'), // almost always esnext
      format: options.format, // almost always 'esm'
      banner: options.banner, // only used in dev
      minify: options.minify ?? true, // only false in dev
      metafile: true,

      plugins: [
        SentryBuildPlugin(definition, LambdaVersion, SentryEnabled),
        NotifyResultPlugin(),
        WriteEsbuildMetaPlugin(MetaFile),
        ValidateLambdaHandler(definition, LambdaFile, options.format),
        ...(options.plugins ?? [])
      ],
      inject: [SentryEnabled ? SentryInit : undefined, ...inject].filter(Boolean) as string[],
      define: {
        ...definedFlags
      }
    };

I'll also add that the SentryInit which is injected when SentryEnabled, is where the initial Sentry.init() call happens.

Otherwise, the lambda code already includes the Sentry wrapper for AWS lambda.

[jd-carroll]

NOTE: For all the esbuild'ers out there, here's a simple plugin to get things working that follows #12233

import fs from 'node:fs';
import path from 'node:path';
import type { Plugin } from 'esbuild';

export function FixImportInTheMiddlePlugin(enabled: boolean): Plugin {
  if (!enabled) {
    return {
      name: 'skip-fix-import-in-the-middle',
      setup() {}
    };
  }

  return {
    name: 'fix-import-in-the-middle',
    setup(build) {
      build.onLoad({ filter: /@opentelemetry\/instrumentation/ }, async (args) => {
        const extension = path.extname(args.path).slice(1);

        let text = await fs.promises.readFile(args.path, 'utf-8');
        if (text.includes('* as ImportInTheMiddle')) {
          text = text.replaceAll(/new\s+(ImportInTheMiddle)\(/gm, 'new $1.default(');
        }

        return {
          contents: text,
          loader: (extension === 'mjs' ? 'js' : extension) as 'ts' | 'js'
        };
      });
    }
  };
}

[AbhiPrasad]

nodejs/import-in-the-middle#88 should fix this with import-in-the-middle, we also have to update OTEL but we'll take care of that too. Appreciate the patience in the meantime @jd-carroll!

[AbhiPrasad]

So this should be resolved with https://github.com/getsentry/sentry-javascript/releases/tag/8.8.0, but please reach out if anything looks off.