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

ESM loader in v11 does not work with @apollo/server package #1777

Closed
barryhagan opened this issue Sep 2, 2023 · 13 comments
Closed

ESM loader in v11 does not work with @apollo/server package #1777

barryhagan opened this issue Sep 2, 2023 · 13 comments
Assignees
Labels

Comments

@barryhagan
Copy link

Description

Attempting to use v11 in an ESM project that includes @apollo/server causes the node process to crash.

In Node v18 you will get a silent exit code 13. In Node v20, it will throw a SyntaxError before exit code 1.

Expected Behavior

Running node --loader newrelic/esm-loader.mjs should work without crashing node.

Steps to Reproduce

Repro is here using import-in-the-middle directly: https://github.com/barryhagan/iitm-apollo-server-repro

You will get the same behavior if you use newrelic/esm-loader.mjs since that is just a re-export of iitm

Your Environment

ESM project
node 18.17.1
newrelic 11.0.0

Additional context

I've opened this issue for import-in-the-middle: nodejs/import-in-the-middle#31

TBD if they can/will fix this or blame @apollo/server

@workato-integration
Copy link

@bizob2828
Copy link
Member

@barryhagan sorry for your troubles. I'll dig into the import-in-the-middle code and see if I can provide a fix for it.

@bizob2828 bizob2828 self-assigned this Sep 11, 2023
@bizob2828
Copy link
Member

I'm going to keep this open even though the fix needs to be made in import-in-the-middle

@bizob2828
Copy link
Member

bizob2828 commented Sep 12, 2023

@barryhagan It's worth noting this problem only exists on Node 20. Workarounds are use Node 16, or 18. Lastly, thhe loader is still experimental from Node.js runtime which in turns means our support is experimental. A fix for this may be within a semver patch/minor.

@barryhagan
Copy link
Author

I am trying to use this with Node18, and that causes exit code 13. We will remain on version 10 of newrelic for now since that works.

@bizob2828
Copy link
Member

bizob2828 commented Sep 12, 2023

The exit code 13 is expected because the loaders CLI flag from Node.js is still experimental. You will see that exit code if you run v10 of agent on Node 18. It is safe to use v11 on Node 16, and 18 just not Node 20 if you're using @apollo/server

@barryhagan
Copy link
Author

Maybe I wasn't clear enough. In a Node18 environment with an ESM project that imports @apollo/server as a dependency, you cannot use the loader from v11 of newrelic. It causes the process to terminate unexpectedly before any code is run. All you get is an exit code 13 from Node. It silently exits even if the first line of code is a console.log statement as shown in this repro

My only choice is to stay on v10 of newrelic right now.

I am mainly surfacing this incompatibility between import-in-the-middle and @apollo/server so others don't have to waste as much time as I did to find out which dependency was the issue or what to look for if upgrading to v11 suddenly causes your project to exit before doing anything. I imagine this is not the only package that causes iitm to fail, as is shown in the trivial example here.

I opened this issue to track nodejs/import-in-the-middle#31 so you can bump your dependency when/if that gets fixed. Until then I will patiently wait in v10.

@bizob2828
Copy link
Member

bizob2828 commented Sep 13, 2023

Ah you're right. So in Node 20 the issue is nodejs/import-in-the-middle#31. Even if this gets fixed, it'll still be broken because of nodejs/import-in-the-middle#32.

@bizob2828
Copy link
Member

There's no work on the agent. We have to wait for import-in-the-middle to release fixes. I offered up help but the maintainers are a little busy atm

@bizob2828 bizob2828 removed their assignment Sep 18, 2023
@jsumners-nr jsumners-nr self-assigned this Dec 7, 2023
@jsumners-nr
Copy link
Contributor

We have a potential solution added in nodejs/import-in-the-middle#43. 🤞 it is acceptable to upstream.

@bizob2828
Copy link
Member

@barryhagan the PR was released as 1.7.2 of import-in-the-middle. I upgraded to the latest and re-ran your repro and it does not crash. I'm closing this issue

@barryhagan
Copy link
Author

I opened this issue to track nodejs/import-in-the-middle#31 so you can bump your dependency when/if that gets fixed.

So, bump your dependency?

@bizob2828
Copy link
Member

@barryhagan we don't bundle dependencies. If you just do a fresh install of the agent, it'll pick up the 1.7.2 version of import-in-the-middle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

3 participants