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 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

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 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

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 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

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

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

@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

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

@stale
Copy link

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 as completed Nov 29, 2020
@jsumners
Copy link

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

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

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?

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

5 participants