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

Add end2end test for compatibility with dd-trace #455

Merged
merged 2 commits into from
Nov 18, 2024
Merged

Conversation

hansott
Copy link
Collaborator

@hansott hansott commented Nov 18, 2024

Fetch in node.js is powered by undici, node requires undici when fetch is called the first time:

let fetchImpl;
// https://fetch.spec.whatwg.org/#fetch-method
ObjectDefineProperty(globalThis, 'fetch', {
  __proto__: null,
  configurable: true,
  enumerable: true,
  writable: true,
  value: function fetch(input, init = undefined) { // eslint-disable-line func-name-matching
    if (!fetchImpl) { // Implement lazy loading of undici module for fetch function
      const undiciModule = require('internal/deps/undici/undici');
      fetchImpl = undiciModule.fetch;
    }
    return fetchImpl(input, init);
  },
});

Only then will setGlobalDispatcher be available (albeit via a symbol hack)

We need to wait until setGlobalDispatcher is properly exposed: nodejs/node#43187

To be able to call setGlobalDispatcher immediately, we call fetch with an arguments and catch the error. We also need to wrap the fetch() call itself, even though there's .catch(...). This is because dd-trace throws an invalid URL error.

Fetch in node.js is powered by undici, node requires undici when fetch
is called the first time:

let fetchImpl;
// https://fetch.spec.whatwg.org/#fetch-method
ObjectDefineProperty(globalThis, 'fetch', {
  __proto__: null,
  configurable: true,
  enumerable: true,
  writable: true,
  value: function fetch(input, init = undefined) { // eslint-disable-line func-name-matching
    if (!fetchImpl) { // Implement lazy loading of undici module for fetch function
      const undiciModule = require('internal/deps/undici/undici');
      fetchImpl = undiciModule.fetch;
    }
    return fetchImpl(input, init);
  },
});

Only then will setGlobalDispatcher be available (albeit via a symbol
hack)

We need to wait until setGlobalDispatcher is properly exposed: nodejs/node#43187

We be able to call setGlobalDispatcher immediately, we call `fetch` with
an arguments and catch the error. We also need to wrap the fetch(<NO
ARGS>) call itself, even though there's `.catch(...)`. This is because
dd-trace throws an invalid URL error.
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
library/sinks/Fetch.ts 50.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@hansott hansott merged commit 264b5c4 into beta Nov 18, 2024
8 of 9 checks passed
@hansott hansott deleted the dd-trace-compat branch November 18, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants