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 a way to retrieve the request object #2171

Closed
thislooksfun opened this issue Mar 11, 2021 · 14 comments · May be fixed by #2750
Closed

Add a way to retrieve the request object #2171

thislooksfun opened this issue Mar 11, 2021 · 14 comments · May be fixed by #2750

Comments

@thislooksfun
Copy link

Context

I'm trying to write some tests to ensure that various meta parameters (query, headers, etc) are set correctly, but it seems that it's impossible to check what triggered an intercepter, only that it did (via isDone()).

It would be extremely useful to have a way to get the actual request that was executed. This is analogous to how most function mocks/spies let you later examine the call history (arguments, this, etc.).

Alternatives

The only alternative I can think of would be to make my own spy from scratch, which I'm not exactly keen to do.

Has the feature been requested before?

Quite possibly, but if it was I was unable to find where.

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

Assuming I can figure out how, sure!

@thislooksfun
Copy link
Author

thislooksfun commented Mar 11, 2021

Some ideas on how to expose this:

Create a Scope.requests variable of type http.ClientRequest[] (or whatever is used under the hood). For example:

const scope = nock("https://example.com")
  .get("foo")
  .reply(200, { bar: "baz" });

scope.done();

expect(scope.requests.length).toEqual(1);
expect(scope.requests[0].getHeader("content-type")).toEqual("application/json");

@blicksky
Copy link

I've been thinking about this from a slightly different perspective. When specifying a request body (https://github.com/nock/nock#specifying-request-body), you can pass a function that returns true if the request should match. If a request is made that doesn't match, nock will throw causing your test to fail, but the error is scoped to the request in general, which doesn't help narrow down which part of the request is the issue. I'm considering using a pattern like the following where I use expect() within this matcher function for each part of the request that I want to assert on, and then always return true:

nock('https://example.com/', {
      reqheaders: {
          'X-Some-Header': (headerValue) => {
              expect(headerValue).toEqual('a');
              return true;
          }
      }
  })
  .post('/path', (requestBody) => {
      expect(requestBody.someRequestBodyField).toEqual('b');
      return true;
  })
  .query((queryString) => {
      expect(queryString.someParam).toEqual('c');
      return true;
  })
  .reply(200, {
      executionArn: mockExecutionArn,
      startDate: mockExecutionStartDate.getTime() / 1000
  });

This works and provides clearer context about which aspect of the request caused the test failure, but it's unfortunate that each component of the request definition has to return true. Perhaps this could be alleviated with a helper higher order function that always returns true. It might be nice if nock's API considered these functions to indicate a match if they simply didn't throw, but that would be a breaking change and wouldn't be as ergonomic for other use cases.

@thislooksfun
Copy link
Author

@blicksky I think that's a perfectly reasonable workaround (and one that I'll probably end up using for the time being), but I still think that having a way to access the requests after the fact is more in line with how other mocks work, and would lead to a cleaner test structure.

@blicksky
Copy link

@thislooksfun Another approach that might help achieve the cleaner test structure you're looking for (using Jest in this example, but possible with other tools as well):

const matchContentTypeHeader = jest.fn().mockReturnValue(true);

nock("https://example.com", {
      reqheaders: {
          'content-type': matchContentTypeHeader
      }
  }))
  .get("foo")
  .reply(200, { bar: "baz" });

expect(matchContentTypeHeader).toHaveBeenCalledTimes(1);
expect(matchContentTypeHeader).toHaveBeenCalledWith("application/json");

I'm not sure if this is what you had in mind when you mentioned not wanting to make your own spy from scratch.

@thislooksfun
Copy link
Author

Turns out that reqheaders and post()'s requestBody were exactly what I needed, thank you! Lesson of the day: read all the documentation, not just some of it.

For future reference, here are the relevant commits:

@mastermatt
Copy link
Member

@blicksky a quick note about your approach of using expect in the body callback. If it works for you, cool, but it won't work for everyone. The issue is that the error is thrown inside the context the the HTTP client and different client handles the unexpected error differently. I don't remember which clients do which, but I know we've had folks report that their client of choice errors out while eating the message making it very difficult to debug. While other (no-so-great) clients will ignore the error completely and go on as if nothing happening.
Unfortunately, for these reasons we've had to denote the use of expect that way a bad practice.
Fear not! Better debugging tools are in the pipe for Nock (no eta 😢).

@blicksky
Copy link

@mastermatt I see, thank you for letting me know!

@thislooksfun
Copy link
Author

Good to know! I think a combination of static rules and the spy approach used here will negate any of my needs to put expect in the callback.

@blicksky
Copy link

I'm going to stick with the static rules for my purposes. The improvement I'm hoping for is for nock's "No match for request" error output to include more details about the differences between the expected and actual requests. @mastermatt Is that what you had in mind when you mentioned better debugging tools? It would be great if nock could at least tell you if it was the request body, headers, or something else that didn't match, but ideally, it would show you a comparison between each unmatched part like Jest does on failed expectations.

@mastermatt
Copy link
Member

There are a few different parts I'm working on. The problem with comparing the exceptions outs of Nock to the likes of Jest or Chai is that in the latter cases those libs know what the expectation is. In the case of Nock, it has a list of interceptors and when a request is detected it filters the interceptors with the hope of finding one where all the "matching" succeeds. Right now I'm working having better debugging to stdout during these matching steps.

@blicksky
Copy link

blicksky commented Apr 6, 2021

Here's another pattern I've been experimenting with that might be helpful:

const requestPromise = new Promise((resolve) => {
  nock('https://example.com/')
    .reply(200)
    .once('request', (request, interceptor, body) => {
        resolve({request, body});
    });
});

await doSomethingThatCausesRequestToBeMade();

const { request, body } = await requestPromise;
expect(request.headers['content-type']).toEqual('text/html');
expect(body).toEqual('foo');

@aukevanleeuwen
Copy link

For other people that are coming along in this thread. My way of doing this would be along these lines. This is TypeScript btw.

/**
 * Helper class to deal with nock expectations and assertions. The idea would be that you
 * can use it as follows:
 * ```
 * const captor = new BodyCaptor();
 *
 * nock('https://example.com').post('/v1/foos', captor.capture()).reply(200);
 * 
 * // something that causes the request to be made
 * 
 * expect(captor.body).toEqual({ foo: 'bar' });
 * ```
 *
 * This - sometimes - makes it easier to figure out what's wrong with the request body
 * rather than just getting a 'No match for request ....'
 */
export class BodyCaptor {
    public body: any; // eslint-disable-line @typescript-eslint/no-explicit-any

    capture() {
        // eslint-disable-next-line @typescript-eslint/no-explicit-any
        return (body: any) => {
            this.body = body;
            return true;
        };
    }
}

@Sleavely
Copy link

It's undocumented and Typescript will complain, but it looks like you can access an array with your requests in scope.interceptors:

const scope = nock(/example\.com/)
  .get(/\/potato/)
  .reply(200);

await fetch('https://example.com/potato/fries');
expect(scope.isDone()).to.equal(true);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const requestedPath = (scope as any).interceptors[0].req.path;
expect(requestedPath).to.equal('/potato/fries');

Tested on nock@13.2.9

@sezanzeb
Copy link

sezanzeb commented Jun 6, 2024

I wish this was a thing and that this issue wouldn't have been closed. Using reqheaders and such is not sufficient for me. I would much rather get a list of requests and do whatever I like with the recorded requests.

sezanzeb pushed a commit to sezanzeb/nock that referenced this issue Jun 6, 2024
sezanzeb pushed a commit to sezanzeb/nock that referenced this issue Jun 6, 2024
sezanzeb pushed a commit to sezanzeb/nock that referenced this issue Sep 9, 2024
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 a pull request may close this issue.

6 participants