Extended got instance needs some defaults reseting in `cloud` env, but not `local` env

[binarymist]

Describe the bug

• Node.js version: 14.15.0
• OS & version: Linux Mint 18

Actual behavior

When run in the local environment, there is no problem, when run in the cloud environment the following values are all retained from the previous request (gotPt.post within requestTestOrTestPlan) unless reset like so:

gotPt.defaults.options.json = undefined;
gotPt.defaults.options.resolveBodyOnly = false;
gotPt.defaults.options.headers['content-type'] = undefined;
gotPt.defaults.options.headers['content-length'] = undefined;

The only thing I can think of is that perhaps my cloud hooks (beforeRequest and/or afterResponse) are somehow affecting this?

I've tried quite a few things in the above mentioned hooks and nothing has changed this behavour so far

Expected behavior

I'd expect that the defaults don't need to be reset in this hacky manner.

Code to reproduce

Code is all at the above links.

I'm not sure this is a code bug, it may be a documentation bug or even me just not understanding how the defaults and/or hooks work?

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

Thanks.

[sindresorhus]

It would be helpful if someone could submit a pull request with a failing tests. That would help to get this fixed faster if it's actually an issue with Got.

[binarymist]

Hi @sindresorhus ...

I guess the first thing to do would be a quick look to see if we're doing something that isn't supposed to be done with got, my first thoughts are around our beforeRequest and afterResponse. Although we've tried different things and been imersed in the got docs for a couple of weeks, so we're leaning toward the following in this order:

1. docs issue, or lack of explanation around this (our) specific use case, although we think we've followed the example patterns correctly?
2. missunderstanding the docs
3. got bug

If it is a got bug, we can attempt to create a failing test. If it's one of the (what we thing would be more likely) first two, then we should probably address them rather than writing a failing test?

Thanks.

[szmarczak]

You misread the docs. https://github.com/sindresorhus/got#hooksafterresponse

-return retryWithMergedOptions(gotPt.defaults.options);
+return retryWithMergedOptions(optionAugmentations);

Got 12 will prevent confusions like this.

[binarymist]

Thanks for your response @szmarczak

That doesn't appear to be the fault, as the retryWithMergedOptions is only invoked on a status code of 401 or 403 which doesn't happen during debugging or most other use cases (I.E. retryWithMergedOptions is never invoked).

In my current testing this line produces an error: RequestError: The 'GET' method cannot be used with a body due to the json body applied in gotPt.post of requestTestOrTestPlan

Could we please keep this issue open until it's resolved?

Thanks.

[szmarczak]

That doesn't appear to be the fault, as the retryWithMergedOptions is only invoked on a status code of 401 or 403 which doesn't happen during debugging or most other use cases (I.E. retryWithMergedOptions is never invoked).

At some point it will fail. Note that this merges the defaults into itself, so you may end up getting e.g. doubled search queries.

In my current testing this line produces an error: RequestError: The 'GET' method cannot be used with a body due to the json body applied in gotPt.post of requestTestOrTestPlan

You're replacing the defaults on each request. Meaning you set the defaults to what the request was. If the request was POST, then suddenly instance.defaults.options.method is POST but it was GET before.

See this simple repro:

const got = require('got');

const gotPt = got.extend({
    hooks: {
        beforeRequest: [
            options => {
                // Because options.method is POST, the default method will also be set to POST.
                // Because options.json is set, it will also be present by default!
                gotPt.defaults.options = got.mergeOptions(gotPt.defaults.options, options);
            }
        ]
    },
    mutableDefaults: true
});

await gotPt.post('https://httpbin.org/anything', {json: {}});

console.log('default method', gotPt.defaults.options.method);
console.log('default json', gotPt.defaults.options.json);

await gotPt.get('https://httpbin.org/anything');

[szmarczak]

Here

https://gitlab.com/purpleteam-labs/purpleteam/-/blob/46fee63515d86909e34946479a0b9713d066c87a/src/presenter/apiDecoratingAdapter.js#L99-101

you do it correctly. But here

https://gitlab.com/purpleteam-labs/purpleteam/-/blob/46fee63515d86909e34946479a0b9713d066c87a/src/presenter/apiDecoratingAdapter.js#L86-88

you're doing it wrong.

[szmarczak]

If you replace

            options.headers.authorization = `Bearer ${await getAccessToken()}`;
            // Save for further requests.
            gotPt.defaults.options = got.mergeOptions(gotPt.defaults.options, options);

with

            options.headers.authorization = `Bearer ${await getAccessToken()}`;
            // Save for further requests.
            gotPt.defaults.headers.authorization = options.headers.authorization;

then it should work.

[binarymist]

Thanks for your time and effort on this one @szmarczak it was much appreciated. I just needed to be pointed in the right directon!

This seems to have fixed it. Took a bit to get it just right, do you see any issues with the change?