Skip to content

Commit

Permalink
Optionally log HTTP responses in HttpStore errors
Browse files Browse the repository at this point in the history 
Summary:
Changelog: [Internal]

Adds a `debug` flag to HttpStore, allowing integrators to opt into including HTTP response bodies in the error messages returned by `get()` / `set()`.

Reviewed By: GijsWeterings

Differential Revision: D55787568

fbshipit-source-id: 5660aeae7be724a07fb1e141788afcfff49db039
  • Loading branch information
@motiz88 @facebook-github-bot
motiz88 authored and facebook-github-bot committed Apr 8, 2024
1 parent f8f7d55 commit c843680
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 19 deletions.
91 changes: 87 additions & 4 deletions packages/metro-cache/src/stores/HttpStore.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ type EndpointOptions = {
params?: URLSearchParams,
headers?: {[string]: string},
additionalSuccessStatuses?: $ReadOnlyArray<number>,
/**
* Whether to include additional debug information in error messages.
*/
debug?: boolean,
};

type Endpoint = {
Expand All @@ -46,6 +50,7 @@ type Endpoint = {
headers?: {[string]: string},
timeout: number,
additionalSuccessStatuses: $ReadOnlySet<number>,
debug: boolean,
};

const ZLIB_OPTIONS = {
Expand Down Expand Up @@ -114,6 +119,7 @@ class HttpStore<T> {
additionalSuccessStatuses: new Set(
options.additionalSuccessStatuses ?? [],
),
debug: options.debug ?? false,
};
}

Expand Down Expand Up @@ -151,8 +157,46 @@ class HttpStore<T> {
code !== 200 &&
!this._getEndpoint.additionalSuccessStatuses.has(code)
) {
res.resume();
reject(new HttpError('HTTP error: ' + code, code));
if (this._getEndpoint.debug) {
res.on('data', chunk => {
data.push(chunk);
});
res.on('error', err => {
reject(
new HttpError(
'Encountered network error (' +
err.message +
') while handling HTTP error: ' +
code +
' ' +
http.STATUS_CODES[code],
code,
),
);
});
res.on('end', () => {
const buffer = Buffer.concat(data);
reject(
new HttpError(
'HTTP error: ' +
code +
' ' +
http.STATUS_CODES[code] +
'\n\n' +
buffer.toString(),
code,
),
);
});
} else {
res.resume();
reject(
new HttpError(
'HTTP error: ' + code + ' ' + http.STATUS_CODES[code],
code,
),
);
}

return;
}
Expand Down Expand Up @@ -227,8 +271,47 @@ class HttpStore<T> {
(code < 200 || code > 299) &&
!this._setEndpoint.additionalSuccessStatuses.has(code)
) {
res.resume();
reject(new HttpError('HTTP error: ' + code, code));
if (this._setEndpoint.debug) {
const data = [];
res.on('data', chunk => {
data.push(chunk);
});
res.on('error', err => {
reject(
new HttpError(
'Encountered network error (' +
err.message +
') while handling HTTP error: ' +
code +
' ' +
http.STATUS_CODES[code],
code,
),
);
});
res.on('end', () => {
const buffer = Buffer.concat(data);
reject(
new HttpError(
'HTTP error: ' +
code +
' ' +
http.STATUS_CODES[code] +
'\n\n' +
buffer.toString(),
code,
),
);
});
} else {
res.resume();
reject(
new HttpError(
'HTTP error: ' + code + ' ' + http.STATUS_CODES[code],
code,
),
);
}

return;
}
Expand Down
16 changes: 3 additions & 13 deletions packages/metro-cache/src/stores/__tests__/HttpGetStore-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @oncall react_native
*/

'use strict';

const {PassThrough} = require('stream');
Expand Down Expand Up @@ -79,7 +69,7 @@ describe('HttpGetStore', () => {

expect(opts.method).toEqual('GET');

callback(responseHttpError(404)); // Page not found
callback(responseHttpError(404));

await promise.then(result => {
expect(result).toBe(null);
Expand All @@ -95,14 +85,14 @@ describe('HttpGetStore', () => {

expect(opts.method).toEqual('GET');

callback(responseHttpError(502)); // Server error
callback(responseHttpError(502));

await promise.then(result => {
expect(result).toBe(null);

expect(warningMessages.length).toBe(1);
expect(warningMessages[0]).toMatchInlineSnapshot(
'"Could not connect to the HTTP cache. Original error: HTTP error: 502"',
`"Could not connect to the HTTP cache. Original error: HTTP error: 502 Bad Gateway"`,
);
});
});
Expand Down
45 changes: 43 additions & 2 deletions packages/metro-cache/src/stores/__tests__/HttpStore-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('HttpStore', () => {
});

process.nextTick(() => {
res.write('HTTP error body');
res.end();
});

Expand Down Expand Up @@ -112,7 +113,27 @@ describe('HttpStore', () => {

promise.catch(err => {
expect(err).toBeInstanceOf(HttpStore.HttpError);
expect(err.message).toMatch(/HTTP error: 503/);
expect(err.message).toMatch(/HTTP error: 503 Service Unavailable/);
expect(err.code).toBe(503);
done();
});
});

it('get() includes HTTP error body in rejection with debug: true', done => {
const store = new HttpStore({endpoint: 'http://example.com', debug: true});
const promise = store.get(Buffer.from('key'));
const [opts, callback] = require('http').request.mock.calls[0];

expect(opts.method).toEqual('GET');

callback(responseHttpError(503));
jest.runAllTimers();

promise.catch(err => {
expect(err).toBeInstanceOf(HttpStore.HttpError);
expect(err.message).toMatch(
/HTTP error: 503 Service Unavailable.*HTTP error body/s,
);
expect(err.code).toBe(503);
done();
});
Expand Down Expand Up @@ -248,7 +269,27 @@ describe('HttpStore', () => {

promise.catch(err => {
expect(err).toBeInstanceOf(HttpStore.HttpError);
expect(err.message).toMatch(/HTTP error: 403/);
expect(err.message).toMatch(/HTTP error: 403 Forbidden/);
expect(err.code).toBe(403);
done();
});
});

it('set() includes HTTP error body in rejection with debug: true', done => {
const store = new HttpStore({endpoint: 'http://example.com', debug: true});
const promise = store.set(Buffer.from('key-set'), {foo: 42});
const [opts, callback] = require('http').request.mock.calls[0];

expect(opts.method).toEqual('PUT');

callback(responseHttpError(403));
jest.runAllTimers();

promise.catch(err => {
expect(err).toBeInstanceOf(HttpStore.HttpError);
expect(err.message).toMatch(
/HTTP error: 403 Forbidden.*HTTP error body/s,
);
expect(err.code).toBe(403);
done();
});
Expand Down

0 comments on commit c843680

Please sign in to comment.