The Wayback Machine - https://web.archive.org/web/20220409145216/https://github.com/nock/nock/issues/1979
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 statusMessage to response #1979

Open
ig3 opened this issue Apr 22, 2020 · 12 comments
Open

Add statusMessage to response #1979

ig3 opened this issue Apr 22, 2020 · 12 comments

Comments

@ig3
Copy link

@ig3 ig3 commented Apr 22, 2020

Context

This isn't really a feature request, as what I need is possible with nock as-is. But I spent several hours searching, reading old issues and searching through the source code to find the solution, so I thought this might help others.

I am testing code that accesses a service that sets the statusMessage of the response, as well as the statusCode. I am using nock to mock the server/service in tests of the client code. Because the service does and the client responds to it, I want to set the statusMessage of the responses provided by nock, as well as the statusCode.

Alternatives

It is possible to set the statusMessage of the response with nock 12.0.3 (and possibly older, but I have only done it with 12.0.3), without any change required. To set the statusMessage of the response, one can use a callback to provide the body of the reply and set the statusMessage via this.req.response. For example:

const scope = nock('http://www.google.com')
  .post('/echo')
  .reply(200, function (uri, requestBody) {
    this.req.response.statusMessage = 'This is the custom status message'
    return {
      status: 200,
      message: 'This is the reply body',
      ...
    }
  })

I have only tried this one form of callback, but I expect the other forms would work as well. As noted in the docs, a function, rather than an arrow function, is necessary to get the correct 'this'.

Has the feature been requested before?

This has been discussed before in #469, #558 and #587 but these are all closed now.

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

No need, as noted above.

Thanks all for a very nice tool!

@paulmelnikow
Copy link
Member

@paulmelnikow paulmelnikow commented Apr 22, 2020

Hi! Are you using a custom status message, or is it that the statusMessage needs to correctly match the error code?

@ig3
Copy link
Author

@ig3 ig3 commented Apr 22, 2020

I am testing code that accesses an API I do not control. The API sets statusMessage to provide information additional to the statusCode (i.e. for a given statusCode, the API will return one of a set of possible status messages, depending on conditions), so the client must use the statusMessage in some cases.

Using nock the usual ways (per examples in the documentation) the statusMessage isn't set in the response - only the statusCode is set. This is fine for most tests, but for this API there are a few cases where the statusMessage makes a difference, so I must set it to exercise those cases in the client under test.

Edit: it occurs to me that I could submit a PR to add an example of setting statusMessage to the documentation. I would be willing to try submitting a PR to do so, if it would be helpful.

@mastermatt
Copy link
Member

@mastermatt mastermatt commented Apr 23, 2020

I'm not opposed to making this available as an option for the reply method, however, it's already a fairly overloaded and complex signature.

I think deciding on the right way to change the API will be the hard part here.

The approach taken in #587 was to allow the status code to be replaced by a plain object instead of a number.
Off the cuff, I'm not a fan because it seems like a caller who wants to supply a status message, body function, and headers will have to call reply in a very convoluted way.

Another option would be to start allowing a single plain object for all the arguments. The object could optionally contain statusCode, statusMessage, headers, and body in all the current ways (string, object, stream, buffer, callback, etc). It could even open up reply to "easily" provide trailers as an option.

Given the addition of object shorthand in ES6, this seems like it would provide a clean API for users who have a complicated ask. eg

const body = fs.createReadStream(filename)
const headers = {...}
nock("...")
  .get("/")
  .reply({ statusCode: 419, statusMessage: "You're a teapot", body, headers })

One followup question if we decide to add status message; should we start setting it on all responses? It would just mean having an lookup object of default messages containing the recommended messages/status-phrases.

@ig3
Copy link
Author

@ig3 ig3 commented Apr 23, 2020

I'm not sure if you are asking me, and I am hesitant to say more. I'm just a user with scant experience. FWIW, my inclination at the moment is to document how to do it with the current API rather than change the API. The documentation already explains how to access the request. It just wasn't obvious to me, until I learned a little more, that that also made the response available. A change to the API can't save me more than a line of code and one somewhat obscure object reference. My requirement to set the statusMessage is a rare requirement, in my experience. I'll leave it to you all to decide what's best and thank you again for a very helpful tool. But if there is something I can do to help, let me know.

@mastermatt
Copy link
Member

@mastermatt mastermatt commented Jun 28, 2020

Unless we decide to update the API in the future, I think this issue should have a new example added to https://github.com/nock/nock/tree/main/examples before its considered resolved.

@jsumners
Copy link

@jsumners jsumners commented Aug 18, 2020

Just having this exact problem and the docs were not helping. Ideally, .reply(400, 'Custom status message') would do the trick. Or .replyWithError(400, 'Custom status message'), but this doesn't send a response in the same manner that I need to test.

@mastermatt
Copy link
Member

@mastermatt mastermatt commented Aug 18, 2020

@jsumners did you try @ig3 example from above?

const scope = nock('http://www.example.com')
  .post('/echo')
  .reply(200, function (uri, requestBody) {
    this.req.response.statusMessage = 'This is the custom status message'
    return {
      status: 200,
      message: 'This is the reply body',
      ...
    }
  })
@jsumners
Copy link

@jsumners jsumners commented Aug 18, 2020

@mastermatt yes, I'm just adding support for that solution being documented at least.

@stale
Copy link

@stale stale bot commented Nov 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.

@stale stale bot added the stale label Nov 21, 2020
@stale stale bot closed this Nov 29, 2020
@jsumners
Copy link

@jsumners jsumners commented Nov 30, 2020

In the Fastify org, we have the stale bot configured to ignore certain labels like "good first issue". Should that be the case here as well?

@mastermatt
Copy link
Member

@mastermatt mastermatt commented Nov 30, 2020

Nock has the "pinned" label for that functionality. But after 100 days without any activity or a PR, there probably isn't a point in leaving this issue open. PRs are still always welcome.

@jsumners
Copy link

@jsumners jsumners commented Nov 30, 2020

So if an issue is labeled as "good first issue", and hasn't been resolved, how will new contributors know that the good starting point still needs work?

@mastermatt mastermatt reopened this Dec 1, 2020
@mastermatt mastermatt added pinned and removed stale labels Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants