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

"ImportInTheMiddle" will crash at run-time #4691

Open
jd-carroll opened this issue May 10, 2024 · 3 comments · May be fixed by #4745
Open

"ImportInTheMiddle" will crash at run-time #4691

jd-carroll opened this issue May 10, 2024 · 3 comments · May be fixed by #4745
Labels
bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc

Comments

@jd-carroll
Copy link

jd-carroll commented May 10, 2024

What happened?

Warning output from esbuild bundler.

▲ [WARNING] Constructing "ImportInTheMiddle" will crash at run-time because it's an import namespace object, not a constructor [call-import-namespace]
    node_modules/@opentelemetry/instrumentation-mongoose/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:259:30:
      259 │ ...     var esmHook = new ImportInTheMiddle([module_2.name], { in...
          ╵                           ~~~~~~~~~~~~~~~~~
  Consider changing "ImportInTheMiddle" to a default import instead:
    node_modules/@opentelemetry/instrumentation-mongoose/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:48:7:
      48 │ import * as ImportInTheMiddle from 'import-in-the-middle';
         │        ~~~~~~~~~~~~~~~~~~~~~~
         ╵        ImportInTheMiddle
6 of 16 warnings shown (disable the message limit with --log-limit=0)
  ...310dc1affb2720b6af2dd368949f880c1da144faa5a6860bc328c2/index.mjs  3.8mb ⚠️
⚡ Done in 144ms

From the code, I would have to agree with the warning that this will crash if used.

I don't understand how you could import like this:
It is not valid to import like this:

And then instantiate like this:

const esmHook =
new (ImportInTheMiddle as unknown as typeof ImportInTheMiddle.default)(
[module.name],
{ internals: false },
<HookFn>hookFn
);

It will absolutely throw an exception.

For ESM, the instantiation would need to look like:

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

But there are lots of things I do not know, so would be interested to hear if this would actually work!

@jd-carroll jd-carroll added bug Something isn't working triage labels May 10, 2024
@jd-carroll
Copy link
Author

@davidmurdoch are you also seeing these warnings?
(ps- sorry for randomly @'ing you on this repo)

@jd-carroll
Copy link
Author

FYI - For ESM this is absolutely a problem.

Using the import syntax as is, the code would need to look like:

import * as ImportInTheMiddle from 'import-in-the-middle';

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

@Ankcorn
Copy link

Ankcorn commented May 13, 2024

Hey, we also have ran into this problem.

I went down a rabbit hole of fixing this across the codebase #4519

But I didn't account for this being a breaking change for some configs.

@dyladan had a proposal that would fix this without the breaking change #4546

I currently don't have the time to do any work on this

@dyladan dyladan added priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc and removed triage labels May 15, 2024
@timfish timfish linked a pull request Jun 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants