Skip to content

Commit

Permalink
Fix shortcut methods giving wrong result
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak committed Apr 15, 2020
1 parent e97cf7e commit 99d70df
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 26 deletions.
6 changes: 6 additions & 0 deletions readme.md
Expand Up @@ -784,6 +784,12 @@ Type: `string | object | Buffer` *(Depending on `options.responseType`)*

The result of the request.

##### rawBody

Type: `Buffer`

The raw result of the request.

##### url

Type: `string`
Expand Down
16 changes: 11 additions & 5 deletions source/as-promise/core.ts
Expand Up @@ -17,18 +17,20 @@ if (!knownHookEvents.includes('beforeRetry' as any)) {
export const knownBodyTypes = ['json', 'buffer', 'text'];

// @ts-ignore The error is: Not all code paths return a value.
export const parseBody = (body: Buffer, response: Response, responseType: ResponseType, encoding?: string): unknown => {
export const parseBody = (response: Response, responseType: ResponseType, encoding?: string): unknown => {
const {rawBody} = response;

try {
if (responseType === 'text') {
return body.toString(encoding);
return rawBody.toString(encoding);
}

if (responseType === 'json') {
return body.length === 0 ? '' : JSON.parse(body.toString()) as unknown;
return rawBody.length === 0 ? '' : JSON.parse(rawBody.toString()) as unknown;
}

if (responseType === 'buffer') {
return Buffer.from(body);
return Buffer.from(rawBody);
}

if (!knownBodyTypes.includes(responseType)) {
Expand All @@ -42,7 +44,6 @@ export const parseBody = (body: Buffer, response: Response, responseType: Respon
export default class PromisableRequest extends Request {
['constructor']: typeof PromisableRequest;
declare options: NormalizedOptions;
declare _throwHttpErrors: boolean;

static normalizeArguments(url?: string | URL, nonNormalizedOptions?: Options, defaults?: Defaults): NormalizedOptions {
const options = super.normalizeArguments(url, nonNormalizedOptions, defaults) as NormalizedOptions;
Expand Down Expand Up @@ -119,6 +120,11 @@ export default class PromisableRequest extends Request {
}
}

// JSON mode
if (options.responseType === 'json' && options.headers.accept === undefined) {
options.headers.accept = 'application/json';
}

return options;
}

Expand Down
31 changes: 15 additions & 16 deletions source/as-promise/index.ts
Expand Up @@ -23,34 +23,28 @@ const proxiedRequestEvents = [

export default function asPromise<T>(options: NormalizedOptions): CancelableRequest<T> {
let retryCount = 0;
let body: Buffer;
let currentResponse: Response;
let globalRequest: PromisableRequest;
let globalResponse: Response;
const emitter = new EventEmitter();

const promise = new PCancelable<T>((resolve, reject, onCancel) => {
const makeRequest = (): void => {
if (options.responseType === 'json' && options.headers.accept === undefined) {
options.headers.accept = 'application/json';
}

// Support retries
// `options.throwHttpErrors` needs to be always true,
// so the HTTP errors are caught and the request is retried.
// The error is **eventaully** thrown
// if the user value `options._throwHttpErrors` is true.
// The error is **eventually** thrown if the user value is true.
const {throwHttpErrors} = options;
if (!throwHttpErrors) {
options.throwHttpErrors = true;
}

const request = new PromisableRequest(options.url, options);
request._throwHttpErrors = throwHttpErrors;
request._noPipe = true;
onCancel(() => request.destroy());

request.once('response', async (response: Response) => {
currentResponse = response;
globalRequest = request;

request.once('response', async (response: Response) => {
response.retryCount = retryCount;

if (response.request.aborted) {
Expand All @@ -66,19 +60,22 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
};

// Download body
let rawBody;
try {
body = await getStream.buffer(request);
rawBody = await getStream.buffer(request);

response.rawBody = rawBody;
} catch (error) {
request._beforeError(new ReadError(error, options, response));
return;
}

// Parse body
try {
response.body = parseBody(body, response, options.responseType, options.encoding);
response.body = parseBody(response, options.responseType, options.encoding);
} catch (error) {
// Fallback to `utf8`
response.body = body.toString('utf8');
response.body = rawBody.toString();

if (isOk()) {
request._beforeError(error);
Expand Down Expand Up @@ -131,6 +128,8 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
return;
}

globalResponse = response;

resolve(options.resolveBodyOnly ? response.body as T : response as unknown as T);
});

Expand Down Expand Up @@ -225,7 +224,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
// Wait until downloading has ended
await promise;

return parseBody(body, currentResponse, responseType);
return parseBody(globalResponse, responseType, options.encoding);
})();

Object.defineProperties(newPromise, Object.getOwnPropertyDescriptors(promise));
Expand All @@ -234,7 +233,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
};

promise.json = () => {
if (body === undefined && options.headers.accept === undefined) {
if (!globalRequest.writableFinished && options.headers.accept === undefined) {
options.headers.accept = 'application/json';
}

Expand Down
12 changes: 7 additions & 5 deletions source/core/index.ts
Expand Up @@ -221,6 +221,7 @@ export interface PlainResponse extends IncomingMessageWithTimings {
// For Promise support
export interface Response<T = unknown> extends PlainResponse {
body: T;
rawBody: Buffer;
retryCount: number;
}

Expand Down Expand Up @@ -1313,11 +1314,12 @@ export default class Request extends Duplex implements RequestEvents<Request> {

try {
const {response} = error as RequestError;
if (response && is.undefined(response.body)) {
response.body = await getStream(response, {
...this.options,
encoding: (this as any)._readableState.encoding
});

if (response) {
response.setEncoding((this as any)._readableState.encoding);

response.rawBody = await getStream.buffer(response);
response.body = response.rawBody.toString();
}
} catch (_) {}

Expand Down
40 changes: 40 additions & 0 deletions test/response-parse.ts
Expand Up @@ -182,3 +182,43 @@ test('shortcuts throw ParseErrors', withServer, async (t, server, got) => {
message: /^Unexpected token o in JSON at position 1 in/
});
});

test('shortcuts result properly when retrying in afterResponse', withServer, async (t, server, got) => {
const nasty = JSON.stringify({hello: 'nasty'});
const proper = JSON.stringify({hello: 'world'});

server.get('/', (request, response) => {
if (request.headers.token === 'unicorn') {
response.end(proper);
} else {
response.statusCode = 401;
response.end(nasty);
}
});

const promise = got({
hooks: {
afterResponse: [
(response, retryWithMergedOptions) => {
if (response.statusCode === 401) {
return retryWithMergedOptions({
headers: {
token: 'unicorn'
}
});
}

return response;
}
]
}
});

const json = await promise.json<{hello: string}>();
const text = await promise.text();
const buffer = await promise.buffer();

t.is(json.hello, 'world');
t.is(text, proper);
t.is(buffer.compare(Buffer.from(proper)), 0);
});

0 comments on commit 99d70df

Please sign in to comment.