-
-
Notifications
You must be signed in to change notification settings - Fork 956
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
Why this "The body
, json
and form
options are mutually exclusive" error?
#1165
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:
|
@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:
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 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 got@10.x.x so it is possibly a regression. |
Thanks for the reproducible code, I'll look into this ASAP |
On the master branch it give this:
which shouldn't happen at all. |
The above test doesn’t fully reproduce my original issue (TypeError instead of a timeout) |
Ok. I'm going to fix this tomorrow, then release |
Indeed this promise throws
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 |
I think it's a 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 |
It's a Node.js bug too https://runkit.com/szmarczak/5e9ff8b7d36ff4001b085053 |
So I luckily found a whole lotta bugs? ;) |
That's right :P |
So I got it throwing 8501c69 |
@szmarczak Thanks for releasing got@11.0.2 I am now able to continue the transition from got@10.x.x to version 11... And I now have your name all over my screen ;)
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 |
It looks like you called |
I did neither of the above. I opened a new issue and try to be as descriptive as can be. |
What would you like to discuss?
I am upgrading my (functional) code from the latest got@10.x.X to got@11.0.1.
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:
got
to query for some CRM data as jsonallowGetBody: true
afterResponse
hook to acquire/refresh oauth token (usinggot
with a POST method with a json payload)afterResponse
hook is executed:The oauth token is correctly acquired/renewed, object
updatedOptions
is correct butretryWithMergedOptions(updatedOptions)
fails withthrow new TypeError('The
body,
jsonand
formoptions are mutually exclusive')
At this point of
_finalizeBody()
,this.option
holds my json filter in bothbody
andjson
property.I am unsure if I originally made a mistake somewhere in the past and must feel lucky my code works in got@10.x.x or if I happen to hit e regression.
Please advise.
Checklist
The text was updated successfully, but these errors were encountered: