Got: Why this "The `body`, `json` and `form` options are mutually exclusive" error?

Created on 20 Apr 2020  Â·  19Comments  Â·  Source: sindresorhus/got

What would you like to discuss?

I am upgrading my (functional) code from the latest [email protected] to [email protected].

Went through the release notes, updated GotError and .pagination... Typescript compiles ok but code breaks.

Not really sure why but I failed at catching any exception occurring within got so I resorted to vscode debugger.

The first got call fails with this sequence sequence:

  1. call got to query for some CRM data as json

    • GET method
    • a json payload defining the filter
    • allowGetBody: true
    • an afterResponse hook to acquire/refresh oauth token (using got with a POST method with a json payload)
  2. since there is no initial oauth token, the afterResponse hook is executed:

    afterResponse: [
      async (response, retryWithMergedOptions) => {
        // Unauthorized
        if (response.statusCode === 401) {
          const updatedOptions: ExtendOptions = { headers: { 'oauth-token': await getNewAuthToken() } };

          // Save for further requests
          authenticatedInstance.defaults.options = got.mergeOptions(
            authenticatedInstance.defaults.options,
            updatedOptions
          );

          return retryWithMergedOptions(updatedOptions);
        }

        // No changes otherwise
        return response;
      },
    ],

The oauth token is correctly acquired/renewed, object updatedOptions is correct but retryWithMergedOptions(updatedOptions) fails with throw new TypeError('Thebody,jsonandformoptions are mutually exclusive')
At this point of _finalizeBody(), this.option holds my json filter in both body and json property.

I am unsure if I originally made a mistake somewhere in the past and must feel lucky my code works in [email protected] or if I happen to hit e regression.

Please advise.

Checklist

  • [x] I have read the documentation.
bug

All 19 comments

@szmarczak FYI I am trying to reproduce & isolate my case. Sharing current code is not a practical options for it would imply divulging some business information beyond my scope of decision ;)

So far it seems that:

  • If I first get a valid oauth token and provide it to the got instance before querying, this partly solves the issue (by obviously avoiding going through the .afterResponse () hook)
  • If I change my queries from got(url, {json: awesome}) to got(url, {body: JSON.stringify(awesome)}) it seems to fix the issue.
  • There are still some of the above TypeError occurring, when using the '.paginate()' method, but I am still investigating this.

@szmarczak Please give a look at this test case:

test('async afterResponse allows to retry with allowGetBody and json payload', withServer, async (t, server, got) => {
    server.get('/', (request, response) => {
        if (request.headers.token !== 'unicorn') {
            response.statusCode = 401;
        }

        response.end();
    });

    const {statusCode} = await got({
        allowGetBody: true,
        json: {hello: 'world'},
        hooks: {
            afterResponse: [
                async (response, retryWithMergedOptions) => {
                    if (response.statusCode === 401) {
                        return retryWithMergedOptions({headers: {token: 'unicorn'}});
                    }

                    return response;
                }
            ]
        }
    });
    t.is(statusCode, 200);
});

It fails unless:

  • you drop the payload json: {hello: 'world'}
  • you make the afterResponse no longer async

Both of those options being a NO-GO for my use since I need a json payload to query the api, and renewing the token implies an await got call.

This may not be the exact problem I am encountering but is like some part of it.

Note that the same code was working in [email protected] so it is possibly a regression.

Thanks for the reproducible code, I'll look into this ASAP

On the master branch it give this:

  ✖ Timed out while running tests

  1 tests were pending in test/hooks.ts

  ◌ async afterResponse allows to retry with allowGetBody and json payload

which shouldn't happen at all.

The above test doesn’t fully reproduce my original issue (TypeError instead of a timeout)
As I was putting up a test case of my own code I ended being stuck by that timeout. My real case is slightly more advanced/complex. I will complete it when there is no longer a timeout (which may share the same root cause as my original issue.)

Ok. I'm going to fix this tomorrow, then release 11.0.2 :)

Indeed this promise throws

RequestError: The body, json and form options are mutually exclusive.

but for some reason the main promise doesn't throw... It's a double bug.

@szmarczak So this is indeed the original problem I identified using the vscode debugger, and it confirms I was unable to catch any thrown error/exception...

It doesn't throw because the error handler is executed only once (as expected). It should raise an uncaught exception error, but it doesn't because there's another error handler attached to the PromisableRequest instance. I don't know why there's a second handler, but I'm looking at it rn.

I think it's a get-stream bug. It uses pump, which uses end-of-stream, which attaches this handler

https://github.com/mafintosh/end-of-stream/blob/e104395e50015a6436d9747b4b1c2a617b267769/index.js#L43-L45

It is not cleaned up for some reason. But it may be a bug in Got as well, dunno.

Indeed, it doesn't throw because of the get-stream bug. The RequestError itself is a Got bug.

https://github.com/sindresorhus/get-stream/issues/36

So I luckily found a whole lotta bugs? ;)

That's right :P

So I got it throwing 8501c6901af4f7c74cd0ae3700b499bb42ce5928

@szmarczak Thanks for releasing [email protected]

I am now able to continue the transition from [email protected] to version 11...

And I now have your name all over my screen ;)

ℹ [CRM-eLease-bridge] says hi…
✖ [ERROR] RequestError: socket hang up
    at ClientRequest.<anonymous> (/Users/guillaumec/rc-dev/rcsf/node_modules/got/dist/source/core/index.js:759:25)
    at Object.onceWrapper (events.js:418:26)
    at ClientRequest.emit (events.js:323:22)
    at ClientRequest.origin.emit (/Users/guillaumec/rc-dev/rcsf/node_modules/@szmarczak/http-timer/dist/source/index.js:39:20)
    at TLSSocket.socketCloseListener (_http_client.js:400:11)
    at TLSSocket.emit (events.js:323:22)
    at net.js:668:12
    at connResetException (internal/errors.js:604:14)
    at TLSSocket.socketCloseListener (_http_client.js:400:25)
    at TLSSocket.emit (events.js:323:22)
    at net.js:668:12
    at TCP.done (_tls_wrap.js:556:7) {
  name: 'RequestError',
  code: 'ECONNRESET',
  timings: {
    start: 1587548453305,
    socket: 1587548453305,
    lookup: undefined,
    connect: undefined,
    secureConnect: undefined,
    upload: undefined,
    response: undefined,
    end: undefined,
    error: 1587548453306,
    abort: 1587548453307,
    phases: {
      wait: 0,
      dns: undefined,
      tcp: undefined,
      tls: undefined,
      request: undefined,
      firstByte: undefined,
      download: undefined,
      total: 2
    }
  }
}
ℹ [CRM-eLease-bridge] waves bye-bye…

Any idea what could be causing this and how I could investigate further? Would you prefer me to open an new issue (maybe in a different repos?)

Thanks in advance.

It's quite similar to #1062 but I'd rather prefer if that was a separate issue, since it's socket hang up and not read ECONNRESET.

It looks like you called request.abort() instead of promise.cancel(), but I'm not sure. A reproducible example would be best.

I did neither of the above. I opened a new issue and try to be as descriptive as can be.
I’ll try to put up a reproducible example when done with home office and daycare duties… Happy Lockdown to all of us.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

quocnguyen picture quocnguyen  Â·  4Comments

f-mer picture f-mer  Â·  4Comments

jamestalmage picture jamestalmage  Â·  3Comments

AxelTerizaki picture AxelTerizaki  Â·  3Comments

astoilkov picture astoilkov  Â·  3Comments