Skip to content

Commit 0f5ec61

Browse files
vzaidmanfacebook-github-bot
authored andcommitted
Improve the error thrown when a bundle fails to load
Summary: Changelog: [General][Breaking] When a bundle was failing to load, throw errors of special type. Differential Revision: D73925027
1 parent e1632da commit 0f5ec61

File tree

3 files changed

+63
-9
lines changed

3 files changed

+63
-9
lines changed

packages/react-native/Libraries/Core/Devtools/__tests__/loadBundleFromServer-test.js

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ jest.mock('../../../Network/RCTNetworking', () => ({
6262
if (name === 'didReceiveNetworkData') {
6363
setImmediate(() => fn([1, mockDataResponse]));
6464
} else if (name === 'didCompleteNetworkResponse') {
65-
setImmediate(() => fn([1, null]));
65+
setImmediate(() => fn([1, mockRequestError]));
6666
} else if (name === 'didReceiveNetworkResponse') {
6767
setImmediate(() => fn([1, null, mockHeaders]));
6868
}
@@ -72,9 +72,14 @@ jest.mock('../../../Network/RCTNetworking', () => ({
7272
}));
7373

7474
let loadBundleFromServer: (bundlePathAndQuery: string) => Promise<void>;
75+
const {
76+
LoadBundleFromServerError,
77+
LoadBundleFromServerRequestError,
78+
} = require('../loadBundleFromServer');
7579

7680
let mockHeaders: {'Content-Type': string};
7781
let mockDataResponse;
82+
let mockRequestError = null;
7883

7984
beforeEach(() => {
8085
jest.resetModules();
@@ -85,15 +90,33 @@ beforeEach(() => {
8590
test('loadBundleFromServer will throw for JSON responses', async () => {
8691
mockHeaders = {'Content-Type': 'application/json'};
8792
mockDataResponse = JSON.stringify({message: 'Error thrown from Metro'});
93+
mockRequestError = null;
94+
95+
try {
96+
await loadBundleFromServer('/Fail.bundle?platform=ios');
97+
} catch (e) {
98+
expect(e).toBeInstanceOf(LoadBundleFromServerError);
99+
expect(e.message).toBe('Error thrown from Metro');
100+
}
101+
});
88102

89-
await expect(
90-
loadBundleFromServer('/Fail.bundle?platform=ios'),
91-
).rejects.toThrow('Error thrown from Metro');
103+
test('loadBundleFromServer will throw LoadBundleFromServerError for request errors', async () => {
104+
mockHeaders = {'Content-Type': 'application/json'};
105+
mockDataResponse = '';
106+
mockRequestError = 'Some error';
107+
108+
try {
109+
await loadBundleFromServer('/Fail.bundle?platform=ios');
110+
} catch (e) {
111+
expect(e).toBeInstanceOf(LoadBundleFromServerRequestError);
112+
expect(e.message).toBe('Some error');
113+
}
92114
});
93115

94116
test('loadBundleFromServer will request a bundle if import bundles are available', async () => {
95117
mockDataResponse = '"code";';
96118
mockHeaders = {'Content-Type': 'application/javascript'};
119+
mockRequestError = null;
97120

98121
await loadBundleFromServer(
99122
'/Banana.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',
@@ -138,6 +161,7 @@ test('loadBundleFromServer will request a bundle if import bundles are available
138161
test('shows and hides the loading view around a request', async () => {
139162
mockDataResponse = '"code";';
140163
mockHeaders = {'Content-Type': 'application/javascript'};
164+
mockRequestError = null;
141165

142166
const promise = loadBundleFromServer(
143167
'/Banana.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',
@@ -157,6 +181,7 @@ test('shows and hides the loading view around a request', async () => {
157181
test('shows and hides the loading view around concurrent requests', async () => {
158182
mockDataResponse = '"code";';
159183
mockHeaders = {'Content-Type': 'application/javascript'};
184+
mockRequestError = null;
160185

161186
const promise1 = loadBundleFromServer(
162187
'/Banana.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',
@@ -178,13 +203,15 @@ test('shows and hides the loading view around concurrent requests', async () =>
178203
test('loadBundleFromServer does not cache errors', async () => {
179204
mockHeaders = {'Content-Type': 'application/json'};
180205
mockDataResponse = JSON.stringify({message: 'Error thrown from Metro'});
206+
mockRequestError = null;
181207

182208
await expect(
183209
loadBundleFromServer('/Fail.bundle?platform=ios'),
184210
).rejects.toThrow();
185211

186212
mockDataResponse = '"code";';
187213
mockHeaders = {'Content-Type': 'application/javascript'};
214+
mockRequestError = null;
188215

189216
await expect(
190217
loadBundleFromServer('/Fail.bundle?platform=ios'),
@@ -194,6 +221,7 @@ test('loadBundleFromServer does not cache errors', async () => {
194221
test('loadBundleFromServer caches successful fetches', async () => {
195222
mockDataResponse = '"code";';
196223
mockHeaders = {'Content-Type': 'application/javascript'};
224+
mockRequestError = null;
197225

198226
const promise1 = loadBundleFromServer(
199227
'/Banana.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',

packages/react-native/Libraries/Core/Devtools/loadBundleFromServer.js

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ let pendingRequests = 0;
2020

2121
const cachedPromisesByUrl = new Map<string, Promise<void>>();
2222

23+
export class LoadBundleFromServerError extends Error {
24+
url: string;
25+
constructor(message: string, url: string): void {
26+
super(message);
27+
this.url = url;
28+
this.name = 'LoadBundleFromServerError';
29+
}
30+
}
31+
32+
export class LoadBundleFromServerRequestError extends LoadBundleFromServerError {
33+
constructor(message: string, url: string): void {
34+
super(message, url);
35+
this.name = 'LoadBundleFromServerRequestError';
36+
}
37+
}
38+
2339
function asyncRequest(
2440
url: string,
2541
): Promise<{body: string, headers: {[string]: string}}> {
@@ -62,10 +78,10 @@ function asyncRequest(
6278
);
6379
completeListener = Networking.addListener(
6480
'didCompleteNetworkResponse',
65-
([requestId, error]) => {
81+
([requestId, errorMessage]) => {
6682
if (requestId === id) {
67-
if (error) {
68-
reject(error);
83+
if (errorMessage) {
84+
reject(new LoadBundleFromServerRequestError(errorMessage, url));
6985
} else {
7086
//$FlowFixMe[incompatible-call]
7187
resolve({body: responseText, headers});
@@ -122,9 +138,10 @@ export default function loadBundleFromServer(
122138
headers['Content-Type'].indexOf('application/json') >= 0
123139
) {
124140
// Errors are returned as JSON.
125-
throw new Error(
141+
throw new LoadBundleFromServerError(
126142
JSON.parse(body).message ||
127143
`Unknown error fetching '${bundlePathAndQuery}'`,
144+
requestUrl,
128145
);
129146
}
130147

packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3843,7 +3843,16 @@ declare export default function getDevServer(): DevServerInfo;
38433843
`;
38443844

38453845
exports[`public API should not change unintentionally Libraries/Core/Devtools/loadBundleFromServer.js 1`] = `
3846-
"declare export default function loadBundleFromServer(
3846+
"declare export class LoadBundleFromServerError extends Error {
3847+
url: string;
3848+
constructor(message: string, url: string): void;
3849+
}
3850+
declare export class LoadBundleFromServerRequestError
3851+
extends LoadBundleFromServerError
3852+
{
3853+
constructor(message: string, url: string): void;
3854+
}
3855+
declare export default function loadBundleFromServer(
38473856
bundlePathAndQuery: string
38483857
): Promise<void>;
38493858
"

0 commit comments

Comments
 (0)