Skip to content

Commit cc5bb43

Browse files
qhanamqhanam
authored
fix: Set appropriate error codes in X-Ray segments (#192)
1 parent 7c9ae11 commit cc5bb43

File tree

6 files changed

+176
-8
lines changed

6 files changed

+176
-8
lines changed

‎src/plugins/event-plugins/FetchPlugin.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@ import {
1616
createXRaySubsegment,
1717
requestInfoToHostname,
1818
addAmznTraceIdHeaderToHeaders,
19-
resourceToUrlString
19+
resourceToUrlString,
20+
is429,
21+
is4xx,
22+
is5xx
2023
} from '../utils/http-utils';
2124
import { HTTP_EVENT_TYPE, XRAY_TRACE_EVENT_TYPE } from '../utils/constant';
2225
import {
2326
errorEventToJsErrorEvent,
2427
isErrorPrimitive
2528
} from '../utils/js-error-utils';
2629
import { HttpEvent } from '../../events/http-event';
27-
import { InternalPlugin } from '../InternalPlugin';
2830

2931
type Fetch = typeof fetch;
3032

@@ -143,14 +145,31 @@ export class FetchPlugin extends MonkeyPatched<Window, 'fetch'> {
143145
xRayTraceEvent.subsegments[0].http.response = {
144146
status: response.status
145147
};
148+
149+
if (is429(response.status)) {
150+
xRayTraceEvent.subsegments[0].throttle = true;
151+
xRayTraceEvent.throttle = true;
152+
} else if (is4xx(response.status)) {
153+
xRayTraceEvent.subsegments[0].error = true;
154+
xRayTraceEvent.error = true;
155+
} else if (is5xx(response.status)) {
156+
xRayTraceEvent.subsegments[0].fault = true;
157+
xRayTraceEvent.fault = true;
158+
}
159+
146160
const cl = parseInt(response.headers.get('Content-Length'), 10);
147161
if (!isNaN(cl)) {
148162
xRayTraceEvent.subsegments[0].http.response.content_length = cl;
149163
}
150164
}
151165

152166
if (error) {
153-
xRayTraceEvent.subsegments[0].error = true;
167+
// Guidance from X-Ray documentation:
168+
// > Record errors in segments when your application returns an
169+
// > error to the user, and in subsegments when a downstream call
170+
// > returns an error.
171+
xRayTraceEvent.fault = true;
172+
xRayTraceEvent.subsegments[0].fault = true;
154173
if (error instanceof Object) {
155174
this.appendErrorCauseFromObject(
156175
xRayTraceEvent.subsegments[0],

‎src/plugins/event-plugins/XhrPlugin.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import {
1111
isUrlAllowed,
1212
HttpPluginConfig,
1313
createXRaySubsegment,
14-
requestInfoToHostname
14+
requestInfoToHostname,
15+
is429,
16+
is4xx,
17+
is5xx
1518
} from '../utils/http-utils';
1619
import { XhrError } from '../../errors/XhrError';
1720
import { HTTP_EVENT_TYPE, XRAY_TRACE_EVENT_TYPE } from '../utils/constant';
@@ -141,6 +144,18 @@ export class XhrPlugin extends MonkeyPatched<XMLHttpRequest, 'send' | 'open'> {
141144
xhrDetails.trace.subsegments[0].http.response = {
142145
status: xhr.status
143146
};
147+
148+
if (is429(xhr.status)) {
149+
xhrDetails.trace.subsegments[0].throttle = true;
150+
xhrDetails.trace.throttle = true;
151+
} else if (is4xx(xhr.status)) {
152+
xhrDetails.trace.subsegments[0].error = true;
153+
xhrDetails.trace.error = true;
154+
} else if (is5xx(xhr.status)) {
155+
xhrDetails.trace.subsegments[0].fault = true;
156+
xhrDetails.trace.fault = true;
157+
}
158+
144159
const cl = parseInt(xhr.getResponseHeader('Content-Length'), 10);
145160
if (!isNaN(cl)) {
146161
xhrDetails.trace.subsegments[0].http.response.content_length = parseInt(
@@ -162,9 +177,14 @@ export class XhrPlugin extends MonkeyPatched<XMLHttpRequest, 'send' | 'open'> {
162177
: xhr.status.toString();
163178
if (xhrDetails) {
164179
const endTime = epochTime();
180+
// Guidance from X-Ray documentation:
181+
// > Record errors in segments when your application returns an
182+
// > error to the user, and in subsegments when a downstream call
183+
// > returns an error.
184+
xhrDetails.trace.fault = true;
165185
xhrDetails.trace.end_time = endTime;
166186
xhrDetails.trace.subsegments[0].end_time = endTime;
167-
xhrDetails.trace.subsegments[0].error = true;
187+
xhrDetails.trace.subsegments[0].fault = true;
168188
xhrDetails.trace.subsegments[0].cause = {
169189
exceptions: [
170190
{

‎src/plugins/event-plugins/__tests__/FetchPlugin.test.ts‎

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import {
1515
mockFetchWith500,
1616
mockFetchWithError,
1717
mockFetchWithErrorObject,
18-
mockFetchWithErrorObjectAndStack
18+
mockFetchWithErrorObjectAndStack,
19+
mockFetchWith400,
20+
mockFetchWith429
1921
} from '../../../test-utils/test-utils';
2022
import { GetSession, PluginContext } from '../../types';
2123
import { XRAY_TRACE_EVENT_TYPE, HTTP_EVENT_TYPE } from '../../utils/constant';
@@ -243,6 +245,7 @@ describe('FetchPlugin tests', () => {
243245
start_time: 0,
244246
trace_id: '1-0-000000000000000000000000',
245247
end_time: 0,
248+
fault: true,
246249
subsegments: [
247250
{
248251
id: '0000000000000000',
@@ -256,7 +259,7 @@ describe('FetchPlugin tests', () => {
256259
traced: true
257260
}
258261
},
259-
error: true,
262+
fault: true,
260263
cause: {
261264
exceptions: [
262265
{
@@ -269,6 +272,96 @@ describe('FetchPlugin tests', () => {
269272
});
270273
});
271274

275+
test('when fetch returns a 404 then segment error is true', async () => {
276+
// Init
277+
global.fetch = mockFetchWith400;
278+
const config: PartialHttpPluginConfig = {
279+
logicalServiceName: 'sample.rum.aws.amazon.com',
280+
urlsToInclude: [/aws\.amazon\.com/]
281+
};
282+
283+
const plugin: FetchPlugin = new FetchPlugin(config);
284+
plugin.load(xRayOnContext);
285+
286+
// Run
287+
await fetch(URL);
288+
plugin.disable();
289+
global.fetch = mockFetch;
290+
291+
// Assert
292+
expect(record.mock.calls[0][1]).toMatchObject({
293+
error: true,
294+
subsegments: [
295+
{
296+
error: true,
297+
http: {
298+
response: { status: 404 }
299+
}
300+
}
301+
]
302+
});
303+
});
304+
305+
test('when fetch returns a 429 then segment throttle is true', async () => {
306+
// Init
307+
global.fetch = mockFetchWith429;
308+
const config: PartialHttpPluginConfig = {
309+
logicalServiceName: 'sample.rum.aws.amazon.com',
310+
urlsToInclude: [/aws\.amazon\.com/]
311+
};
312+
313+
const plugin: FetchPlugin = new FetchPlugin(config);
314+
plugin.load(xRayOnContext);
315+
316+
// Run
317+
await fetch(URL);
318+
plugin.disable();
319+
global.fetch = mockFetch;
320+
321+
// Assert
322+
expect(record.mock.calls[0][1]).toMatchObject({
323+
throttle: true,
324+
subsegments: [
325+
{
326+
throttle: true,
327+
http: {
328+
response: { status: 429 }
329+
}
330+
}
331+
]
332+
});
333+
});
334+
335+
test('when fetch returns a 500 then segment fault is true', async () => {
336+
// Init
337+
global.fetch = mockFetchWith500;
338+
const config: PartialHttpPluginConfig = {
339+
logicalServiceName: 'sample.rum.aws.amazon.com',
340+
urlsToInclude: [/aws\.amazon\.com/]
341+
};
342+
343+
const plugin: FetchPlugin = new FetchPlugin(config);
344+
plugin.load(xRayOnContext);
345+
346+
// Run
347+
await fetch(URL);
348+
plugin.disable();
349+
global.fetch = mockFetch;
350+
351+
// Assert
352+
expect(record.mock.calls[0][1]).toMatchObject({
353+
fault: true,
354+
subsegments: [
355+
{
356+
fault: true,
357+
http: {
358+
response: { status: 500 }
359+
}
360+
}
361+
]
362+
});
363+
});
364+
272365
test('when plugin is disabled then the plugin does not record a trace', async () => {
273366
// Init
274367
const config: PartialHttpPluginConfig = {

‎src/plugins/event-plugins/__tests__/XhrPlugin.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,13 @@ describe('XhrPlugin tests', () => {
208208
start_time: 0,
209209
trace_id: '1-0-000000000000000000000000',
210210
end_time: 0,
211+
fault: true,
211212
subsegments: [
212213
{
213214
id: '0000000000000000',
214215
start_time: 0,
215216
end_time: 0,
217+
fault: true,
216218
http: {
217219
request: {
218220
method: 'GET',

‎src/plugins/utils/http-utils.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,18 @@ export const defaultConfig: HttpPluginConfig = {
5353
addXRayTraceIdHeader: false
5454
};
5555

56+
export const is4xx = (status: number) => {
57+
return Math.floor(status / 100) === 4;
58+
};
59+
60+
export const is5xx = (status: number) => {
61+
return Math.floor(status / 100) === 5;
62+
};
63+
64+
export const is429 = (status: number) => {
65+
return status === 429;
66+
};
67+
5668
export const isUrlAllowed = (url: string, config: HttpPluginConfig) => {
5769
const include = config.urlsToInclude.some((urlPattern) =>
5870
urlPattern.test(url)

‎src/test-utils/test-utils.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,29 @@ export const mockFetchWith500 = jest.fn(
165165
Promise.resolve({
166166
status: 500,
167167
statusText: 'InternalError',
168-
headers: {},
168+
headers: new Headers({ 'Content-Length': '125' }),
169+
body: '',
170+
ok: false
171+
} as any)
172+
);
173+
174+
export const mockFetchWith400 = jest.fn(
175+
(input: RequestInfo, init?: RequestInit): Promise<Response> =>
176+
Promise.resolve({
177+
status: 404,
178+
statusText: 'Not Found',
179+
headers: new Headers({ 'Content-Length': '125' }),
180+
body: '',
181+
ok: false
182+
} as any)
183+
);
184+
185+
export const mockFetchWith429 = jest.fn(
186+
(input: RequestInfo, init?: RequestInit): Promise<Response> =>
187+
Promise.resolve({
188+
status: 429,
189+
statusText: 'Too Many Requests',
190+
headers: new Headers({ 'Content-Length': '125' }),
169191
body: '',
170192
ok: false
171193
} as any)

0 commit comments

Comments
 (0)