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

fix: ESM hooking with Node.js v18.19.0 #3785

Closed
wants to merge 1 commit into from

Conversation

trentm
Copy link
Member

@trentm trentm commented Dec 11, 2023

import-in-the-middle@1.6.0 fixes an issue with double-importing
with node 18.19.0 (due to a backport of ESM loading being done
off main thread).

Fixes: #3784

import-in-the-middle@1.6.0 fixes an issue with double-importing
with node 18.19.0 (due to a backport of ESM loading being done
off main thread).

Fixes: #3784
@trentm trentm self-assigned this Dec 11, 2023
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Dec 11, 2023
@elastic-apm-tech elastic-apm-tech added this to In Progress in APM-Agents (OLD) Dec 11, 2023
@trentm
Copy link
Member Author

trentm commented Dec 11, 2023

Sigh. Now we are hitting the other issue in IITM when used with Node.js versions that use off-main-thread ESM loading:
nodejs/import-in-the-middle#29

E.g. from https://github.com/elastic/apm-agent-nodejs/actions/runs/7171406915/job/19526333837?pr=3785

# simple SQS V3 with ESM (fixtures/use-client-sqs.mjs)
not ok 20 fixtures/use-client-sqs.mjs exited successfully: err=Error: Command failed: /opt/hostedtoolcache/node/18.19.0/x64/bin/node fixtures/use-client-sqs.mjs (node:9069) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`: --import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("../../../../loader.mjs", pathToFileURL("./"));' (Use `node --trace-warnings ...` to show where the warning was created) file:///home/runner/work/apm-agent-nodejs/apm-agent-nodejs/test/instrumentation/modules/@aws-sdk/fixtures/use-client-sqs.mjs:19 CreateQueueCommand, ^^^^^^^^^^^^^^^^^^ SyntaxError: The requested module '@aws-sdk/client-sqs' does not provide an export named 'CreateQueueCommand' at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21) at async ModuleJob.run (node:internal/modules/esm/module_job:191:5) at async ModuleLoader.import (node:internal/modules/esm/loader:336:24) at async loadESM (node:internal/process/esm_loader:34:7) at async handleMainPromise (node:internal/modules/run_main:106:12) 
  ---
    operator: error
    at: done (/home/runner/work/apm-agent-nodejs/apm-agent-nodejs/test/_utils.js:353:17)
    stack: |-
      Error: Command failed: /opt/hostedtoolcache/node/18.19.0/x64/bin/node fixtures/use-client-sqs.mjs
      (node:9069) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
      --import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("../../../../loader.mjs", pathToFileURL("./"));'
      (Use `node --trace-warnings ...` to show where the warning was created)
      file:///home/runner/work/apm-agent-nodejs/apm-agent-nodejs/test/instrumentation/modules/@aws-sdk/fixtures/use-client-sqs.mjs:19
        CreateQueueCommand,
        ^^^^^^^^^^^^^^^^^^
      SyntaxError: The requested module '@aws-sdk/client-sqs' does not provide an export named 'CreateQueueCommand'
          at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
          at async ModuleJob.run (node:internal/modules/esm/module_job:191:5)
          at async ModuleLoader.import (node:internal/modules/esm/loader:336:24)
       

@trentm
Copy link
Member Author

trentm commented Dec 11, 2023

Ultimately we'll want a fix for that issue -- there is an in-progres PR for that: nodejs/import-in-the-middle#30

Either we sit on this PR, or we could workaround this second issue and avoid testing the affected subset of modules with node v18.19.0. @aws-sdk/client-sqs is one of those modules.

@david-luna david-luna self-requested a review December 11, 2023 21:30
@david-luna
Copy link
Member

Either we sit on this PR, or we could workaround this second issue and avoid testing the affected subset of modules with node v18.19.0. @aws-sdk/client-sqs is one of those modules.

As discussed let's make workaround until nodejs/import-in-the-middle#30 is merged and published.

@trentm
Copy link
Member Author

trentm commented Dec 14, 2023

There was a recent IITM commit (nodejs/import-in-the-middle#43) with what I think is a fix for this. It is a very different impl of the issue. I haven't yet had a chance to test whether that fixes the same issue. Also, it isn't yet in an IITM release.

@trentm
Copy link
Member Author

trentm commented Feb 2, 2024

Superseded by #3857

@trentm trentm closed this Feb 2, 2024
APM-Agents (OLD) automation moved this from In Progress to Done Feb 2, 2024
@trentm trentm deleted the trentm/fix-esm-node18.19.0 branch February 5, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
Development

Successfully merging this pull request may close these issues.

ESM instrumentation/hooking fails with Node.js v18.19.0
2 participants