Skip to content
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

Got RequestError when explicitly set responseType to undefined #1252

Closed
2 tasks done
nguyenlc1993 opened this issue May 12, 2020 · 4 comments
Closed
2 tasks done

Got RequestError when explicitly set responseType to undefined #1252

nguyenlc1993 opened this issue May 12, 2020 · 4 comments
Labels
bug Something does not work as it should regression Something does not work anymore

Comments

@nguyenlc1993
Copy link

nguyenlc1993 commented May 12, 2020

Describe the bug

  • Node.js version: v12.16.2
  • OS & version: Ubuntu 18.04

The project at my company uses got-fetch library, which uses got as peer dependency to create fetch-like function. Starting with got v11.1.1, we encounter this error when using got-fetch:

RequestError: Unknown body type 'undefined' in "https://www.google.com/" in "https://www.google.com/"
    at Object.exports.parseBody (/home/nguyen/Desktop/got-test/node_modules/got/dist/source/as-promise/core.js:29:15)
    at PromisableRequest.<anonymous> (/home/nguyen/Desktop/got-test/node_modules/got/dist/source/as-promise/index.js:78:44)
    at Object.exports.parseBody (/home/nguyen/Desktop/got-test/node_modules/got/dist/source/as-promise/core.js:23:15)
    at PromisableRequest.<anonymous> (/home/nguyen/Desktop/got-test/node_modules/got/dist/source/as-promise/index.js:78:44)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

After investigating the source code, we found that got-fetch sets responseType to be undefined and parses the response body by itself.

got-fetch/src/lib/fetch.ts:53

    const gotOpts: GotOptions = {
      // url needs to be stringified to support UNIX domain sockets, and
      // For more info see https://github.com/alexghr/got-fetch/pull/8
      url: url.toString(),
      searchParams,
      followRedirect: true,
      throwHttpErrors: false,
      method: (request.method as any) || 'get',
      isStream: false,
      resolveBodyOnly: false,
      // we'll do our own response parsing in `GotFetchResponse`
      responseType: undefined
    };

When we run test code with responseType explicitly set to undefined, we also got the same behavior. This behavior does not happen in got v11.1.0 and before.

Actual behavior

Got throws RequestError: Unknown body type 'undefined'.

Expected behavior

Got throws no error and return response as normal.

Code to reproduce

const got = require('got');

(async function() {
    const response = await got('https://www.google.com', {
        responseType: undefined,
    });
    console.log(response.body);
})();

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@nguyenlc1993 nguyenlc1993 changed the title Got error when explicitly set responseType to undefined Got RequestError when explicitly set responseType to undefined May 12, 2020
@Giotino
Copy link
Collaborator

Giotino commented May 12, 2020

According to the documentation responseType must be a string ('text', 'json' or 'buffer').

That behavior is due to this code:
https://github.com/sindresorhus/got/blob/9770e540d088c35ec0b467fa3d2fafb60ed6797d/source/as-promise/core.ts#L23,L42

@nguyenlc1993
Copy link
Author

According to the documentation responseType must be a string ('text', 'json' or 'buffer').

That behavior is due to this code:
https://github.com/sindresorhus/got/blob/9770e540d088c35ec0b467fa3d2fafb60ed6797d/source/as-promise/core.ts#L23,L42

Thank you, I know that undefined is not a valid value for responseType, and also know the code that caused the behavior. I just don't understand why this behavior did not happen in earlier versions, as well as why responseType not default to text when set to undefined.
The issue caused us not be able to update got to latest version, so I wonder if the old behavior can be restored.

@Giotino
Copy link
Collaborator

Giotino commented May 12, 2020

I just don't understand why this behavior did not happen in earlier versions

In Got v10 there was this line after the merge (between default values and user options)

options.responseType = options.responseType ?? 'text';

That took care of defaulting responseType to 'text' event if it was overwritten by the user options as undefined.

as well as why responseType not default to text when set to undefined.

This is because responseType , documentation wise, cannot be undefined.

I don't mean to be rude but you where using it wrong, even the Got v10 documentation states that it must be

Type: string

@szmarczak szmarczak added bug Something does not work as it should regression Something does not work anymore labels May 12, 2020
@szmarczak
Copy link
Collaborator

Options are optional, so when you pass anything as undefined it should not throw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should regression Something does not work anymore
Projects
None yet
Development

No branches or pull requests

3 participants