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

[Node v21.7.0] Impossible to stub fetch #2590

Open
regseb opened this issue Mar 8, 2024 · 3 comments
Open

[Node v21.7.0] Impossible to stub fetch #2590

regseb opened this issue Mar 8, 2024 · 3 comments

Comments

@regseb
Copy link

regseb commented Mar 8, 2024

Describe the bug
With Node.js v21.7.0, it's no longer possible to stub the fetch() function.

Edit:

To Reproduce

  • package.json

    {
      "name": "testcase",
      "version": "1.0.0",
      "type": "module",
      "devDependencies": {
        "sinon": "17.0.1"
      }
    }
  • testcase.js

    import sinon from "sinon";
    
    const stub = sinon
        .stub(globalThis, "fetch")
        .resolves(Response.json({ foo: "bar" }));
    
    const response = await fetch("https://baz.qux/");
    const json = await response.json();
    
    console.log(json);
    console.log(stub.firstCall.firstArg);

Steps to reproduce the behavior:

  1. npm install
  2. node testcase.js
node:internal/deps/undici/undici:13737
      Error.captureStackTrace(err, this);
            ^

TypeError: fetch failed
    at node:internal/deps/undici/undici:13737:13
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async file:///home/sregne/script/tc/testcase.js:7:18 {
  [cause]: Error: getaddrinfo ENOTFOUND baz.qux
      at GetAddrInfoReqWrap.onlookupall [as oncomplete] (node:dns:118:26) {
    errno: -3008,
    code: 'ENOTFOUND',
    syscall: 'getaddrinfo',
    hostname: 'baz.qux'
  }
}

Node.js v21.7.0

Expected behavior

{ foo: 'bar' }
https://baz.qux/

Context (please complete the following information):

  • Sinon version : 17.0.1
  • Runtime: Node.js v21.7.0

Additional context
It's work with Node.js v21.6.2.


The problem seems to come from Node.js and the pull request nodejs/node#51598 (comment)

This was a breaking change for tests mocking global fetch, for ex:

import { describe, it } from 'node:test';
import assert from 'node:assert/strict';
import { fetchSomething } from '../lib/index.mjs';

describe('my test suite', () => {
  it('fetch stuff', async (t) => {
    const mockValue = { key: 'value' };
    const mockFetch = async () => ({
      json: async () => mockValue,
      status: 200,
    });

    t.mock.method(global, 'fetch', mockFetch);
    assert.deepStrictEqual(await fetchSomething(), mockValue);
    t.mock.restoreAll();
  });
});

which produces:

✖ fetch stuff (0.894292ms)
  TypeError [ERR_INVALID_ARG_VALUE]: The argument 'methodName' must be a method. Received undefined
      at MockTracker.method (node:internal/test_runner/mock/mock:241:13)
      at TestContext.<anonymous> (my.test.mjs:13:12)
      at Test.runInAsyncScope (node:async_hooks:206:9)
      at Test.run (node:internal/test_runner/test:641:25)
      at Suite.processPendingSubtests (node:internal/test_runner/test:382:18)
      at Test.postRun (node:internal/test_runner/test:732:19)
      at Test.run (node:internal/test_runner/test:690:12)
      at async Promise.all (index 0)
      at async Suite.run (node:internal/test_runner/test:966:7)
      at async startSubtest (node:internal/test_runner/harness:218:3) {
    code: 'ERR_INVALID_ARG_VALUE'
  }

to fix the break, i had to reference global.fetch before mocking it...ie:

fetch;
t.mock.method(global, 'fetch', mockFetch);
@fatso83
Copy link
Contributor

fatso83 commented Mar 14, 2024

Thank you for linking up the associated Node issues. Lots of good stuff there, including your own contributions. I think it's wise to hold off for a little while in order to see if this partly resolves itself within Node (my take after reading the discussion).

@iantocristian
Copy link

iantocristian commented Apr 4, 2024

Same behaviour in node 20.12.0 (stopped working in this version)

@fatso83
Copy link
Contributor

fatso83 commented Apr 4, 2024

We can't do much about this until nodejs/node#52275 (created 5 days ago) is merged and then subsequently released and possibly backmerged (into version 20).

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

No branches or pull requests

3 participants