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 18+: make nock work native fetch #2397

Open
1 of 2 tasks
johnb8005 opened this issue Sep 10, 2022 · 69 comments · Fixed by #2588 · May be fixed by #2517
Open
1 of 2 tasks

Node 18+: make nock work native fetch #2397

johnb8005 opened this issue Sep 10, 2022 · 69 comments · Fixed by #2588 · May be fixed by #2517
Labels

Comments

@johnb8005
Copy link

Please avoid duplicates

Context

Since node v18, fetch is now natively available and one does not require to install node-fetch or the like anymore. I have been using the native fetch quite successfully however I can't use it with nock

Alternatives

No response

If the feature request is accepted, would you be willing to submit a PR?

  • yes
@mastermatt
Copy link
Member

mastermatt commented Sep 13, 2022

Dup of #2183 and #2336

For now, set --no-experiemental-fetch flag

@Lewiscowles1986
Copy link

TBH neither of those directly mention Nock. Would it be possible to add fetch, node18+, and undici as labels?

I Think it might help prevent the need to mark things as duplicate.

@dzek69
Copy link

dzek69 commented Mar 1, 2023

If upgrading nock to work with native fetch requires some time then at least add something about that to README.

Common issues mentions axios for example and it should mention fetch as its popularity will probably only grow.

@mattccrampton
Copy link

Do we have any updates to this? Would love to keep using nock but this is breaking our flow now.

@gr2m
Copy link
Member

gr2m commented May 27, 2023

If upgrading nock to work with native fetch requires some time then at least add something about that to README.

That's a good idea. Thoughts @mastermatt? If someone could get a pull request started that would be great.

@mastermatt
Copy link
Member

It should absolutely be documented in the Readme.

@gr2m
Copy link
Member

gr2m commented May 30, 2023

For now we can recommend to set the --no-experiemental-fetch flag to make it work again.

For what it's worth, my net interceptor library (https://github.com/gr2m/node-net-interceptor) does intercept native fetch as it intercepts at the net/tls level (while nock is intercepting at the http module level which fetch does not use), so there is hope.

If anyone would like to dig into https://github.com/gr2m/node-net-interceptor and native fetch and see how to successfully intercept a simple GET request, that would be a great start.

Here is a starting point. Intercepting works, but I haven't digged into how the response needs to look like for it to successfully mock a full request lifecycle for the native fetch

import netInterceptor from "@gr2m/net-interceptor";

netInterceptor.start();
netInterceptor.on("connect", (socket, options, bypass) => {
  console.log("intercepted!");
  // bypass();
});

netInterceptor.on("connection", (socket) => {
  socket.write(
    `HTTP/1.1 200 OK
Content-Type: text/html; charset=UTF-8
Content-Encoding: UTF-8
Accept-Ranges: bytes
Connection: keep-alive

works`
  );

  socket.end();
});

const response = await fetch("http://example.com");
console.log(await response.text());

Update: see working example below.

@gr2m gr2m changed the title Make it work with the native fetch function Node 18+: make nock work native fetch May 30, 2023
@dzek69
Copy link

dzek69 commented May 30, 2023

For now we can recommend to set the --no-experiemental-fetch flag to make it work again.

This could only help if one is not using native fetch by desire. Disabling native fetch just makes my code not working at all :)

@gr2m
Copy link
Member

gr2m commented May 30, 2023

good point. I mostly work in projects where node-fetch is used which uses the native fetch when available and falls back to a custom implementation.

I know it's not ideal, but as a temporary workaround, could you for testing use node-fetch and make it the global fetch method as described here?
https://github.com/node-fetch/node-fetch#providing-global-access

@wujekbogdan
Copy link

The best solution I found so far is the msw library. It works with any HTTP library, including the native fetch. Here's what I came up with:

import { SetupServer, setupServer } from 'msw/node';
import { rest } from 'msw';
import { myApiClient } from './my-api-client';

type ServerOptions = Parameters<typeof setupServer>;

const withRequestInterception =
  (handlers: ServerOptions, test: (server: SetupServer) => any) => async () => {
    const server = setupServer(...handlers);
    server.listen();

    return Promise.resolve(test(server)).finally(() => {
      server.resetHandlers();
      server.close();
    });
  };

describe('myApiClient', () => {
  it(
    'should work!',
    withRequestInterception(
      [
        rest.get('https://my-mocked.url', (req, res, ctx) =>
          res(ctx.status(200, 'Mocked status'))
        ),
      ],
      async () => {
        const response = await myApiClient('https://my-mocked.url');
        expect(response.status).toEqual(200);
        expect(response.statusText).toEqual('Mocked status');
      }
    )
  );
});

@RichardJECooke
Copy link

RichardJECooke commented Jul 10, 2023

Please state that nock does not work with fetch right at the top of your readme.md. Even the most basic nock example fails with the most common way of calling sites - fetch. I've wasted an hour before I realised this expected feature doesn't work:

async function fetchExample() {
  nock("http://example.com").get("/").reply(200, "Mocked response");
  try {
    const response = await fetch("http://example.com");
    const data = await response.text();
    console.log(data);
  } catch (error) {
    console.error("An error occurred during the fetch request:", error);
  }
}

fetchExample();

@gr2m gr2m pinned this issue Jul 11, 2023
@gr2m
Copy link
Member

gr2m commented Jul 11, 2023

I added a warning in dd15ba5

@isaaxite

This comment was marked as duplicate.

KeesCBakker added a commit to KeesCBakker/hubot-grafana that referenced this issue Aug 7, 2023
When testing the nock package does not know how to behave with the native fetch, so we use 'node-fetch' to make fetch testable with nock. More info: nock/nock#2397 (comment)
@kddsultan
Copy link

@gr2m, are there any plans to support this in near feature? Nock is a great testing library...

@gr2m
Copy link
Member

gr2m commented Aug 10, 2023

I'm not able to work on it by myself right now, I just don't have the time. See my comment at #2397 (comment). I'd be happy to onboard a new co-maintainer if someone wants to take this on

@kddsultan
Copy link

I'm not able to work on it by myself right now, I just don't have the time. See my comment at #2397 (comment). I'd be happy to onboard a new co-maintainer if someone wants to take this on

OK. Let me at least try... How can we do the onboarding?

@mikicho
Copy link
Contributor

mikicho commented Feb 24, 2024

@holzerch Fixed in #2591 (14.0.0-beta.4)

@holzerch
Copy link

@mikicho works perfectly. Thanks very much! Now only recording support is missing to fully migrate

@aannoune
Copy link

aannoune commented Mar 7, 2024

Hello there, I'm trying this fetch beta feature for a couple of weeks now.
It works fine in most cases but I noticed that I'm not able to simulate timeouts with nock on fetch requests.
the delai() method is not taken into account.
Am i missing something ?

@kettanaito
Copy link

@aannoune, fetch by itself doesn't support timeouts. If Nock ships an API that helps you mock a request timeout, then Nock has to provide that support explicitly by using an AbortController and a custom timeout logic that triggers .abort() when the timeout is reached.

oscard0m added a commit to oscard0m/example-probot-vercel-ts that referenced this issue Mar 7, 2024
@Uzlopak
Copy link
Member

Uzlopak commented Mar 7, 2024

@kettanaito
Why AbortController? Delay can also be about delaying a request for a specific time till return a 200 OK.

@oscard0m
Copy link

oscard0m commented Mar 7, 2024

The beta is working nicely for uvu + probot. Great job team!

oscard0m added a commit to oscard0m/example-probot-vercel-ts that referenced this issue Mar 7, 2024
* fix(deps): update dependency probot to v13

* fix(app): use Probot logger properly

* build(deps): tests need to use nock@14-beta

details here: nock/nock#2397

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Oscar Dominguez <dominguez.celada@gmail.com>
@kettanaito
Copy link

@Ugzuzg, the original question was about timeouts so that's what I assumed. You are right, for delaying responses you just have to take the mock delay into account, it's a different thing.

@aannoune
Copy link

aannoune commented Mar 7, 2024

@Ugzuzg : i confirm that my primary goal is to trigger a timeout error in fetch request to test the handling of a request falling into timeout. I there any AbortController support planned in Nock ?

@mikicho
Copy link
Contributor

mikicho commented Mar 7, 2024

@aannoune Can you please share the code? (nock interceptor + fetch request)

@aannoune
Copy link

aannoune commented Mar 7, 2024

@mikicho i cannot share the full code of the fetch request (it is not my own code but another squad's lib i must use) but it is called with a
{signal: AbortSignal.timeout(5000)} in fetch options.

regarding the interceptor : it is a rather simple one like
nock("https://fakUrl").get(':myPass').delay(10000).reply(500)
that i would expect to make my fetch call return a timeout error

@levinmr
Copy link

levinmr commented Mar 27, 2024

This is great. I'd love to see fetch support added to recording as well.

matteo-cristino added a commit to dyne/slangroom that referenced this issue Mar 28, 2024
puria pushed a commit to dyne/slangroom that referenced this issue Mar 28, 2024
* refactor(http): use node-fetch instead of axios in http plugin

Add also response header to result returned by salngroom that now is contains the fields: status, result, headers

* refactor(http): use nock@beta in tests since latest stable nock does not support fetch

nock/nock#2397
@mikicho
Copy link
Contributor

mikicho commented Apr 10, 2024

@aannoune After some thinking, it seems like the "fake delay" feature does not make sense in the same way it does with the http module API.
Meanwhile, we recommend you use time fakers to mimic the same behavior.

@mkurapov
Copy link

Hi @mikicho, thanks for adding support for native fetch!

I haven't been able to pinpoint the actual problem yet on my side (I don't think this is a nock problem per se, so I didn't want to make an issue), but when trying to use nock in my jest tests, I get an error when nock tries to patch global.fetch:

TypeError: Cannot assign to read only property 'fetch' of object '[object global]'

The output of Object.getOwnPropertyDescriptor(global, 'fetch') shows that it's not writable to begin with:

    {
      value: [Function: fetch],
      writable: false,
      enumerable: true,
      configurable: true
    }

Have you seen properties of global not being writable by default when doing patching of the global object in nock?

I also wanted to add to this discussion since it's possible someone else has run into this/will run into this.

@mikicho
Copy link
Contributor

mikicho commented Apr 20, 2024

@mkurapov Interesting. Is there something else that may override fetch before Nock does?
image

adrienjoly added a commit to adrienjoly/telegram-scribe-bot that referenced this issue Apr 21, 2024
…k not intercepting calls to github with FAKE_CREDS

solution: nock/nock#2397 (comment)
adrienjoly added a commit to adrienjoly/telegram-scribe-bot that referenced this issue Apr 21, 2024
…ha (#63)

* fix(deps): migrate to node.js 20

* fix: use node:test instead of mocha

* fix: `SyntaxError: Cannot use import statement outside a module` on `make test`

* fix(tests): `RequestError [HttpError]: Bad credentials` cause but nock not intercepting calls to github with FAKE_CREDS
solution: nock/nock#2397 (comment)

* fix(tests): make all tests run

* $ npm i -D firebase-tools@latest

* bump version to 1.7.1
@0x33dm
Copy link

0x33dm commented Apr 23, 2024

This is great. I'd love to see fetch support added to recording as well.

Recording is well needed. Like when you nocking a 3rd party library and you don't even know which requests are happening, recording is the only way to go.

Unfortunately at this moment on the project I'm working nock is just recording an empty array :(

@mkurapov
Copy link

@mikicho it was a result of having an older version of jest. I was on jest 29.5.0, but upgrading to 29.7.0 fixes the problem.

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