Fixes a bug where args were being overridden by previous requests when using requestOptionsTransport #450
Conversation
…clone so that requests don't mutate future requests
Codecov Report
@@ Coverage Diff @@
## master #450 +/- ##
==========================================
+ Coverage 89.1% 89.12% +0.01%
==========================================
Files 44 44
Lines 1258 1260 +2
Branches 190 190
==========================================
+ Hits 1121 1123 +2
Misses 137 137
Continue to review full report at Codecov.
|
| @@ -64,7 +65,8 @@ var proxiedRequestTransport = function proxiedRequestTransport(proxyURL) { | |||
|
|
|||
| var requestOptionsTransport = function requestOptionsTransport(options) { | |||
| return function _requestOptionsTransport(args, cb) { | |||
| var requestArgs = defaults(options, getRequestTransportArgs(args)); | |||
| var instanceOptions = cloneDeep(options); | |||
There was a problem hiding this comment.
could this line be outside of the inner _requestOptionsTransport function? or in other words, what's the point in cloning the options each time a request is fired, as opposed to just cloning it once per transport "constructed"?
There was a problem hiding this comment.
I think the problem is that the way defaults works is by putting instanceOptions as the base of requestArgs and then layering the args on top of it. This means that after the args are layered on top, instanceOptions has now been overridden with the args specified by the caller.
This is problematic for the next request because then instanceOptions now also includes the args from the previous request. By ensuring that the original options object doesn't mutate and cloning it for each request, we avoid this issue.
There was a problem hiding this comment.
ah! i was assuming defaults() is a pure function. silly lodash developers. 👍
Summary
There's a bug when using the requestOptionsTransport where the options object starts storing arguments from previous requests. This makes a clone of the object so that even if it's mutated, a new one is used for each request.
Requirements (place an
xin each[ ])