fix: Interceptor.reply to allow callback and raw headers. #1517
Conversation
Fixes nock#1513. Ensures raw headers are still returned when using the form `reply(callback, rawHeaders)`.
mastermatt
force-pushed
the
bug/reply-allow-callback-and-headers
branch
from
7c51d97
to
2249dda
Compare
|
fyi @paulmelnikow |
| @@ -453,7 +454,7 @@ Interceptor.prototype.basicAuth = function basicAuth(options) { | |||
| /** | |||
| * Set query strings for the interceptor | |||
| * @name query | |||
| * @param Object Object of query string name,values (accepts regexp values) | |||
| * @param queries Object of query string name,values (accepts regexp values) | |||
| @@ -495,7 +496,7 @@ Interceptor.prototype.query = function query(queries) { | |||
| /** | |||
| * Set number of times will repeat the interceptor | |||
| * @name times | |||
| * @param Integer Number of times to repeat (should be > 0) | |||
| * @param newCounter Number of times to repeat (should be > 0) | |||
| .reply( | ||
| () => [202, 'body', { 'x-key': 'value', 'x-key-2': 'value 2 override' }], | ||
| { 'x-key-2': 'value 2', 'x-key-3': 'value 3' } | ||
| ) |
There was a problem hiding this comment.
Oh no! I didn't realize it was possible to pass the headers to the callback. That reverses what I wrote at #1513 (comment). I'm sorry for my misunderstanding!
I do not think we need to provide so many different ways of doing this. 😝
What about supporting these forms:
.reply(() => 'body').reply(() => ['body', { 'x-key': 'value', 'x-key-2': 'value 2 override' }]).reply(() => [202, 'body', { 'x-key': 'value', 'x-key-2': 'value 2 override' }]).reply((uri, requestBody, cb) => cb(null, 'body'), 1000)).reply((uri, requestBody, cb) => cb(null, ['body', { 'x-key': 'value', 'x-key-2': 'value 2 override' }]), 1000)).reply((uri, requestBody, cb) => cb(null, [201, 'body', { 'x-key': 'value', 'x-key-2': 'value 2 override' }]), 1000))
That way the function and async forms have effectively the same interfaces as the synchronous calls to .reply().
Ideally all these paths would be tested, and the documentation would make it clear what is possible.
Thoughts?
There was a problem hiding this comment.
I don't think my issue/question/change relates to the sync vs async forms of the callback.
I thinks it's more about a confusion behind the intended signature of the method, regardless of what the callbadk returns..
My original assumption was the intended signature was the two forms below. This seems to closely match how I interpret the docs as they stand today.
.replay(statusCode, [bodyOrCallback, [rawHeaders]])
.replay(callback)
If that is correct, I think we should update the beginning of the method to the following
if (_.isFunction(statusCode)) {
body = statusCode
statusCode = 200
rawHeaders = undefined
}The arguments.length <= 2 is confusing/misleading, and clearing the rawHeaders clarifies the method's intention to anyone digging around, plus reduces the changes for bugs in the future.
That being said, this PR does not do this.
Instead it makes the signature of the method more like this
.replay([statusCode], [bodyOrCallback, [rawHeaders]])
or maybe it's better described as
.replay(statusCode, [bodyOrCallback, [rawHeaders]])
.replay(callback, [rawHeaders])
Personally, I'm less of a fan of having all these ways of accomplishing the same thing, so I'd be happy to close this PR. I just want to make sure we're on the same page.
There was a problem hiding this comment.
My original assumption was the intended signature was the two forms below. This seems to closely match how I interpret the docs as they stand today.
.replay(statusCode, [bodyOrCallback, [rawHeaders]])
.replay(callback)
Yes, I agree this is a better interface.
Personally, I'm less of a fan of having all these ways of accomplishing the same thing, so I'd be happy to close this PR. I just want to make sure we're on the same page.
Sounds good. Thanks!
|
@mastermatt Thanks so much for this! I realized from reading your tests that I misunderstood what was currently possible with our API. Left you a comment; happy to discuss! |
Fixes #1513.
Ensures raw headers are still returned when using the form
reply(callback, rawHeaders).