Skip to content

Commit 6384004

Browse files
alexbrazieralexbrazier
committed
fix: handle null/undefined responses and improve error handling in XHR interceptor
1 parent 442e48d commit 6384004

4 files changed

Lines changed: 311 additions & 81 deletions

File tree

‎src/NetworkRequestInfo.ts‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,11 @@ export default class NetworkRequestInfo {
108108
}
109109

110110
private async parseResponseBlob() {
111-
const blobReader = new BlobFileReader();
112-
blobReader.readAsText(this.response);
111+
if (this.response === null || this.response === undefined) {
112+
return '';
113+
}
113114

115+
const blobReader = new BlobFileReader();
114116
return await new Promise<string>((resolve, reject) => {
115117
const handleError = () => reject(blobReader.error);
116118

@@ -119,14 +121,32 @@ export default class NetworkRequestInfo {
119121
});
120122
blobReader.addEventListener('error', handleError);
121123
blobReader.addEventListener('abort', handleError);
124+
125+
try {
126+
blobReader.readAsText(this.response);
127+
} catch (error) {
128+
reject(error);
129+
}
122130
});
123131
}
124132

125133
async getResponseBody() {
126-
const body = await (this.responseType !== 'blob'
127-
? this.response
128-
: this.parseResponseBlob());
134+
if (this.endTime === 0 && this.status < 0) {
135+
return 'Pending response...';
136+
}
129137

130-
return this.stringifyFormat(body);
138+
try {
139+
const body = await (this.responseType !== 'blob'
140+
? this.response
141+
: this.parseResponseBlob());
142+
143+
if (body === '' || body === null || body === undefined) {
144+
return '';
145+
}
146+
147+
return this.stringifyFormat(body);
148+
} catch {
149+
return '[Unable to load response body]';
150+
}
131151
}
132152
}

‎src/XHRInterceptor.spec.ts‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
type ReadyStateListener = () => void;
2+
3+
class FakeXMLHttpRequest {
4+
static readonly UNSENT = 0;
5+
static readonly OPENED = 1;
6+
static readonly HEADERS_RECEIVED = 2;
7+
static readonly LOADING = 3;
8+
static readonly DONE = 4;
9+
10+
readyState = FakeXMLHttpRequest.UNSENT;
11+
response: any = '';
12+
responseText = '';
13+
responseType = '';
14+
responseURL = '';
15+
status = 0;
16+
timeout = 0;
17+
onreadystatechange = null;
18+
19+
openCalled = false;
20+
sendCalled = false;
21+
setRequestHeaderCalled = false;
22+
23+
private listeners: Record<string, ReadyStateListener[]> = {};
24+
private responseHeaders: Record<string, string> = {};
25+
26+
open(
27+
_method?: string,
28+
_url?: string,
29+
_async?: boolean,
30+
_username?: string | null,
31+
_password?: string | null
32+
) {
33+
this.openCalled = true;
34+
}
35+
36+
send(_data?: any) {
37+
this.sendCalled = true;
38+
}
39+
40+
setRequestHeader(_header?: string, _value?: string) {
41+
this.setRequestHeaderCalled = true;
42+
}
43+
44+
getResponseHeader(name: string): string | null {
45+
return this.responseHeaders[name.toLowerCase()] || null;
46+
}
47+
48+
getAllResponseHeaders(): string {
49+
return Object.entries(this.responseHeaders)
50+
.map(([name, value]) => `${name}: ${value}`)
51+
.join('\r\n');
52+
}
53+
54+
addEventListener(type: string, listener: ReadyStateListener) {
55+
if (!this.listeners[type]) {
56+
this.listeners[type] = [];
57+
}
58+
this.listeners[type].push(listener.bind(this));
59+
}
60+
61+
setResponseHeaders(headers: Record<string, string>) {
62+
this.responseHeaders = headers;
63+
}
64+
65+
emitReadyState(state: number) {
66+
this.readyState = state;
67+
(this.listeners.readystatechange || []).forEach((listener) => listener());
68+
}
69+
}
70+
71+
const loadInterceptor = () => {
72+
jest.resetModules();
73+
return require('./XHRInterceptor').default;
74+
};
75+
76+
describe('XHRInterceptor', () => {
77+
beforeEach(() => {
78+
(global as any).XMLHttpRequest = FakeXMLHttpRequest;
79+
});
80+
81+
it('does not block XHR methods when interceptor callbacks throw', () => {
82+
const interceptor = loadInterceptor();
83+
84+
interceptor.setOpenCallback(() => {
85+
throw new Error('open failed');
86+
});
87+
interceptor.setRequestHeaderCallback(() => {
88+
throw new Error('header failed');
89+
});
90+
interceptor.setSendCallback(() => {
91+
throw new Error('send failed');
92+
});
93+
94+
interceptor.enableInterception();
95+
96+
const xhr = new FakeXMLHttpRequest();
97+
expect(() => xhr.open('GET', 'https://example.com')).not.toThrow();
98+
expect(() => xhr.setRequestHeader('x-test', '1')).not.toThrow();
99+
expect(() => xhr.send('data')).not.toThrow();
100+
101+
expect(xhr.openCalled).toBe(true);
102+
expect(xhr.setRequestHeaderCalled).toBe(true);
103+
expect(xhr.sendCalled).toBe(true);
104+
105+
interceptor.disableInterception();
106+
});
107+
108+
it('does not require addEventListener to exist', () => {
109+
const interceptor = loadInterceptor();
110+
interceptor.enableInterception();
111+
112+
const xhr = new FakeXMLHttpRequest() as any;
113+
xhr.addEventListener = undefined;
114+
115+
expect(() => xhr.open('GET', 'https://example.com')).not.toThrow();
116+
expect(() => xhr.send()).not.toThrow();
117+
expect(xhr.sendCalled).toBe(true);
118+
119+
interceptor.disableInterception();
120+
});
121+
122+
it('passes non-text response objects through callback for blob responses', () => {
123+
const interceptor = loadInterceptor();
124+
const responseCallback = jest.fn();
125+
interceptor.setResponseCallback(responseCallback);
126+
interceptor.enableInterception();
127+
128+
const xhr = new FakeXMLHttpRequest();
129+
const responseBlob = { _data: 'blob' };
130+
131+
xhr.responseType = 'blob';
132+
xhr.response = responseBlob;
133+
xhr.status = 200;
134+
xhr.timeout = 123;
135+
xhr.responseURL = 'https://example.com';
136+
137+
xhr.open('GET', 'https://example.com');
138+
xhr.send();
139+
xhr.emitReadyState(FakeXMLHttpRequest.DONE);
140+
141+
expect(responseCallback).toHaveBeenCalledTimes(1);
142+
expect(responseCallback.mock.calls[0][2]).toBe(responseBlob);
143+
144+
interceptor.disableInterception();
145+
});
146+
147+
it('invokes header and response callbacks once each', () => {
148+
const interceptor = loadInterceptor();
149+
const headerReceivedCallback = jest.fn();
150+
const responseCallback = jest.fn();
151+
interceptor.setHeaderReceivedCallback(headerReceivedCallback);
152+
interceptor.setResponseCallback(responseCallback);
153+
interceptor.enableInterception();
154+
155+
const xhr = new FakeXMLHttpRequest();
156+
xhr.setResponseHeaders({
157+
'content-type': 'application/json; charset=utf-8',
158+
'content-length': '42',
159+
});
160+
xhr.responseType = 'text';
161+
xhr.responseText = '{"ok":true}';
162+
163+
xhr.open('GET', 'https://example.com');
164+
xhr.send();
165+
166+
xhr.emitReadyState(FakeXMLHttpRequest.HEADERS_RECEIVED);
167+
xhr.emitReadyState(FakeXMLHttpRequest.HEADERS_RECEIVED);
168+
xhr.emitReadyState(FakeXMLHttpRequest.DONE);
169+
xhr.emitReadyState(FakeXMLHttpRequest.DONE);
170+
171+
expect(headerReceivedCallback).toHaveBeenCalledTimes(1);
172+
expect(responseCallback).toHaveBeenCalledTimes(1);
173+
174+
interceptor.disableInterception();
175+
});
176+
});

‎src/XHRInterceptor.ts‎

Lines changed: 71 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,11 @@ function enableInterception(): void {
100100
url: url.toString(),
101101
};
102102

103-
openCallback(method, url.toString(), this);
103+
try {
104+
openCallback(method, url.toString(), this);
105+
} catch {
106+
// Interceptor callbacks must not break network requests.
107+
}
104108

105109
return originalXHROpen!.call(
106110
this,
@@ -117,7 +121,11 @@ function enableInterception(): void {
117121
header: string,
118122
value: string
119123
): void {
120-
requestHeaderCallback(header, value, this);
124+
try {
125+
requestHeaderCallback(header, value, this);
126+
} catch {
127+
// Interceptor callbacks must not break network requests.
128+
}
121129
return originalXHRSetRequestHeader!.call(this, header, value);
122130
};
123131

@@ -126,64 +134,78 @@ function enableInterception(): void {
126134
const xhr = this as any;
127135

128136
const dataString = body === null || body === undefined ? '' : String(body);
129-
sendCallback(dataString, xhr);
137+
try {
138+
sendCallback(dataString, xhr);
139+
} catch {
140+
// Interceptor callbacks must not break network requests.
141+
}
130142

131143
// Use addEventListener which is more reliable than overriding onreadystatechange
132144
// This works even when handlers are set after send() is called
133-
this.addEventListener('readystatechange', function () {
134-
if (this.readyState === XMLHttpRequest.HEADERS_RECEIVED) {
135-
if (!xhr._interception?.hasCalledHeaderReceived) {
136-
if (xhr._interception) {
137-
xhr._interception.hasCalledHeaderReceived = true;
145+
if (typeof this.addEventListener === 'function') {
146+
this.addEventListener('readystatechange', function () {
147+
if (this.readyState === XMLHttpRequest.HEADERS_RECEIVED) {
148+
if (!xhr._interception?.hasCalledHeaderReceived) {
149+
if (xhr._interception) {
150+
xhr._interception.hasCalledHeaderReceived = true;
151+
}
152+
const contentType = this.getResponseHeader('content-type') || '';
153+
const contentLength = this.getResponseHeader('content-length');
154+
const responseSize = contentLength
155+
? parseInt(contentLength, 10)
156+
: 0;
157+
const responseHeaders = parseResponseHeaders(
158+
this.getAllResponseHeaders()
159+
);
160+
161+
// Set responseHeaders on xhr for compatibility with Logger.ts
162+
xhr.responseHeaders = responseHeaders;
163+
164+
try {
165+
headerReceivedCallback(
166+
contentType,
167+
responseSize,
168+
responseHeaders,
169+
xhr
170+
);
171+
} catch {
172+
// Interceptor callbacks must not break network requests.
173+
}
138174
}
139-
const contentType = this.getResponseHeader('content-type') || '';
140-
const contentLength = this.getResponseHeader('content-length');
141-
const responseSize = contentLength ? parseInt(contentLength, 10) : 0;
142-
const responseHeaders = parseResponseHeaders(
143-
this.getAllResponseHeaders()
144-
);
145-
146-
// Set responseHeaders on xhr for compatibility with Logger.ts
147-
xhr.responseHeaders = responseHeaders;
148-
149-
headerReceivedCallback(
150-
contentType,
151-
responseSize,
152-
responseHeaders,
153-
xhr
154-
);
155175
}
156-
}
157176

158-
if (this.readyState === XMLHttpRequest.DONE) {
159-
if (!xhr._interception?.hasCalledResponse) {
160-
if (xhr._interception) {
161-
xhr._interception.hasCalledResponse = true;
162-
}
163-
let responseData = '';
164-
if (this.responseType === '' || this.responseType === 'text') {
165-
responseData = this.responseText || '';
166-
} else if (this.responseType === 'json' && this.response) {
177+
if (this.readyState === XMLHttpRequest.DONE) {
178+
if (!xhr._interception?.hasCalledResponse) {
179+
if (xhr._interception) {
180+
xhr._interception.hasCalledResponse = true;
181+
}
182+
let responseData: any = this.response;
183+
if (this.responseType === '' || this.responseType === 'text') {
184+
responseData = this.responseText || '';
185+
} else if (this.responseType === 'json' && this.response) {
186+
try {
187+
responseData = JSON.stringify(this.response);
188+
} catch {
189+
responseData = '[Unable to stringify response]';
190+
}
191+
}
192+
167193
try {
168-
responseData = JSON.stringify(this.response);
194+
responseCallback(
195+
this.status,
196+
this.timeout,
197+
responseData,
198+
this.responseURL,
199+
this.responseType,
200+
xhr
201+
);
169202
} catch {
170-
responseData = '[Unable to stringify response]';
203+
// Interceptor callbacks must not break network requests.
171204
}
172-
} else if (this.response) {
173-
responseData = '[Non-text response]';
174205
}
175-
176-
responseCallback(
177-
this.status,
178-
this.timeout,
179-
responseData,
180-
this.responseURL,
181-
this.responseType,
182-
xhr
183-
);
184206
}
185-
}
186-
});
207+
});
208+
}
187209

188210
return originalXHRSend!.call(this, body);
189211
};

0 commit comments

Comments
 (0)