From 9b6103755325ad68710f435bd7c41b15459e14df Mon Sep 17 00:00:00 2001 From: Kenan Yusuf Date: Mon, 6 Nov 2023 10:55:25 +0000 Subject: [PATCH] Handle aborted requests (#158) Co-authored-by: Kevin Paxton --- .changeset/spotty-hats-learn.md | 8 +++++ packages/core/src/fetch.test.ts | 26 +++++++++++++- packages/core/src/fetch.ts | 15 +++++++- packages/node/src/fetch.ts | 24 +++++++++++-- packages/web/src/fetch.ts | 25 +++++++++++-- .../src/components/ui/TraceDetail.test.tsx | 24 +++++++++++++ .../webui/src/components/ui/TraceDetail.tsx | 7 +++- .../src/components/ui/TraceListRow.test.tsx | 19 ++++++++++ .../webui/src/components/ui/TraceListRow.tsx | 22 +++++++++--- packages/webui/src/testing/mockTraces/rest.ts | 36 ++++++++++++++++++- 10 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 .changeset/spotty-hats-learn.md diff --git a/.changeset/spotty-hats-learn.md b/.changeset/spotty-hats-learn.md new file mode 100644 index 0000000..ee3604f --- /dev/null +++ b/.changeset/spotty-hats-learn.md @@ -0,0 +1,8 @@ +--- +'@envyjs/webui': minor +'@envyjs/core': minor +'@envyjs/node': minor +'@envyjs/web': minor +--- + +Handle aborted requests diff --git a/packages/core/src/fetch.test.ts b/packages/core/src/fetch.test.ts index 9e585b0..eb9aba0 100644 --- a/packages/core/src/fetch.test.ts +++ b/packages/core/src/fetch.test.ts @@ -1,4 +1,5 @@ import { + getEventFromAbortedFetchRequest, getEventFromFetchRequest, getEventFromFetchResponse, getUrlFromFetchRequest, @@ -96,7 +97,7 @@ describe('fetch', () => { }); }); - describe('getEventFromFetchRequest', () => { + describe('getEventFromFetchResponse', () => { it('should map a fetch response', async () => { const request = getEventFromFetchRequest('1', 'http://localhost/api/path'); const response = await getEventFromFetchResponse(request, { @@ -132,6 +133,29 @@ describe('fetch', () => { }); }); + describe('getEventFromAbortedFetchRequest', () => { + it('should map an aborted fetch request', () => { + const request = getEventFromFetchRequest('1', 'http://localhost/api/path'); + const response = getEventFromAbortedFetchRequest(request, 100); + + expect(response).toEqual({ + http: { + host: 'localhost', + method: 'GET', + path: '/api/path', + port: NaN, + requestHeaders: {}, + state: 'aborted', + url: 'http://localhost/api/path', + duration: 100, + }, + id: '1', + parentId: undefined, + timestamp: expect.any(Number), + }); + }); + }); + describe('getUrlFromFetchRequest', () => { it('should parse a Request type', () => { const info = new MockRequest('http://localhost/api/path'); diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index d62443a..a8425ae 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -26,13 +26,26 @@ export function getEventFromFetchRequest(id: string, input: RequestInfo | URL, i }; } +/** + * Returns an {@link Event} from an aborted fetch request + */ +export function getEventFromAbortedFetchRequest(req: Event, duration: number): Event { + return { + ...req, + http: { + ...req.http!, + state: HttpRequestState.Aborted, + duration, + }, + }; +} + /** * Returns an {@link Event} from a fetch response */ export async function getEventFromFetchResponse(req: Event, response: Response): Promise { return { ...req, - http: { ...req.http!, state: HttpRequestState.Received, diff --git a/packages/node/src/fetch.ts b/packages/node/src/fetch.ts index f310b20..3c99dcf 100644 --- a/packages/node/src/fetch.ts +++ b/packages/node/src/fetch.ts @@ -1,4 +1,9 @@ -import { Plugin, getEventFromFetchRequest, getEventFromFetchResponse } from '@envyjs/core'; +import { + Plugin, + getEventFromAbortedFetchRequest, + getEventFromFetchRequest, + getEventFromFetchResponse, +} from '@envyjs/core'; import { generateId } from './id'; @@ -7,13 +12,28 @@ export const Fetch: Plugin = (_options, exporter) => { global.fetch = async (...args) => { const id = generateId(); const startTs = performance.now(); + let response: Response | undefined = undefined; // export the initial request data const reqEvent = getEventFromFetchRequest(id, ...args); exporter.send(reqEvent); + // if the args contain a signal, listen for abort events + const signal = args[1]?.signal; + if (signal) { + signal.addEventListener('abort', () => { + // if the request has already been resolved, we don't need to do anything + if (response) return; + + // export the aborted request data + const duration = performance.now() - startTs; + const abortEvent = getEventFromAbortedFetchRequest(reqEvent, duration); + exporter.send(abortEvent); + }); + } + // execute the actual request - const response = await originalFetch(...args); + response = await originalFetch(...args); const responseClone = response.clone(); const resEvent = await getEventFromFetchResponse(reqEvent, responseClone); diff --git a/packages/web/src/fetch.ts b/packages/web/src/fetch.ts index fe4f2cc..13a86f6 100644 --- a/packages/web/src/fetch.ts +++ b/packages/web/src/fetch.ts @@ -1,4 +1,9 @@ -import { Plugin, getEventFromFetchRequest, getEventFromFetchResponse } from '@envyjs/core'; +import { + Plugin, + getEventFromAbortedFetchRequest, + getEventFromFetchRequest, + getEventFromFetchResponse, +} from '@envyjs/core'; import { generateId } from './id'; import { calculateTiming } from './performance'; @@ -8,6 +13,7 @@ export const Fetch: Plugin = (_options, exporter) => { window.fetch = async (...args) => { const id = generateId(); const startTs = performance.now(); + let response: Response | undefined = undefined; // export the initial request data const reqEvent = getEventFromFetchRequest(id, ...args); @@ -15,8 +21,23 @@ export const Fetch: Plugin = (_options, exporter) => { performance.mark(reqEvent.id, { detail: { type: 'start' } }); + // if the args contain a signal, listen for abort events + const signal = args[1]?.signal; + if (signal) { + signal.addEventListener('abort', () => { + // if the request has already been resolved, we don't need to do anything + if (response) return; + + // export the aborted request data + const duration = performance.now() - startTs; + const abortEvent = getEventFromAbortedFetchRequest(reqEvent, duration); + exporter.send(abortEvent); + }); + } + // execute the actual request - const response = await originalFetch(...args); + response = await originalFetch(...args); + const responseClone = response.clone(); const resEvent = await getEventFromFetchResponse(reqEvent, responseClone); diff --git a/packages/webui/src/components/ui/TraceDetail.test.tsx b/packages/webui/src/components/ui/TraceDetail.test.tsx index e83c0d8..93db2e8 100644 --- a/packages/webui/src/components/ui/TraceDetail.test.tsx +++ b/packages/webui/src/components/ui/TraceDetail.test.tsx @@ -191,6 +191,30 @@ describe('TraceDetail', () => { expect(status).not.toBeInTheDocument(); }); + it('should display aborted indicator when request is aborted', () => { + const mockAbortedTrace = mockTraces.find(trace => trace.http?.state === HttpRequestState.Aborted); + getSelectedTraceFn.mockReturnValue(mockAbortedTrace); + + const { getByTestId } = render(); + + const summary = getByTestId('summary'); + const abortedIndicator = within(summary).queryByTestId('aborted-indicator'); + + expect(abortedIndicator).toBeInTheDocument(); + }); + + it('should not display aborted indicator when response is received', () => { + const mockAbortedTrace = mockTraces.find(trace => trace.http?.state === HttpRequestState.Received); + getSelectedTraceFn.mockReturnValue(mockAbortedTrace); + + const { getByTestId } = render(); + + const summary = getByTestId('summary'); + const abortedIndicator = within(summary).queryByTestId('aborted-indicator'); + + expect(abortedIndicator).not.toBeInTheDocument(); + }); + it('should display full URL', () => { getSelectedTraceFn.mockReturnValue({ ...mockTrace, diff --git a/packages/webui/src/components/ui/TraceDetail.tsx b/packages/webui/src/components/ui/TraceDetail.tsx index 619f242..07b895d 100644 --- a/packages/webui/src/components/ui/TraceDetail.tsx +++ b/packages/webui/src/components/ui/TraceDetail.tsx @@ -62,7 +62,7 @@ export default function TraceDetail() { const { http, serviceName, timestamp } = trace || {}; const { method, host, url, requestHeaders, statusCode, statusMessage, responseHeaders, duration, state } = http || {}; - + const requestAborted = state === HttpRequestState.Aborted; const responseComplete = state !== HttpRequestState.Sent; const updateTimer = useCallback(() => { @@ -102,6 +102,11 @@ export default function TraceDetail() { {method} + {requestAborted && ( + + Aborted + + )} {statusCode && ( {httpStatusLabel} diff --git a/packages/webui/src/components/ui/TraceListRow.test.tsx b/packages/webui/src/components/ui/TraceListRow.test.tsx index 60bd4f7..ae44c41 100644 --- a/packages/webui/src/components/ui/TraceListRow.test.tsx +++ b/packages/webui/src/components/ui/TraceListRow.test.tsx @@ -65,6 +65,25 @@ describe('TraceListRow', () => { expect(statusCodeData).toHaveTextContent('204'); }); + it('should display HTTP method and aborted status if response exists and state is aborted', () => { + const trace = { + id: '1', + timestamp: 0, + http: { + method: 'POST', + statusCode: 204, + state: 'aborted', + } as Trace['http'], + }; + + const { getByTestId } = render(); + const methodData = getByTestId('column-data-method-cell'); + const statusCodeData = getByTestId('column-data-code-cell'); + + expect(methodData).toHaveTextContent('POST'); + expect(statusCodeData).toHaveTextContent('Aborted'); + }); + it('should render nothing for method and status if `http` property of trace is not defined', () => { const trace = { id: '1', diff --git a/packages/webui/src/components/ui/TraceListRow.tsx b/packages/webui/src/components/ui/TraceListRow.tsx index a500604..620e697 100644 --- a/packages/webui/src/components/ui/TraceListRow.tsx +++ b/packages/webui/src/components/ui/TraceListRow.tsx @@ -1,3 +1,5 @@ +import { HttpRequestState } from '@envyjs/core'; + import { Loading } from '@/components'; import useApplication from '@/hooks/useApplication'; import { ListDataComponent } from '@/systems'; @@ -27,8 +29,8 @@ export default function TraceListRow({ trace }: { trace: Trace }) {
{trace.http?.method.toUpperCase()}
-
- {trace.http?.statusCode} +
+ {getResponseStatus(trace)}
@@ -41,8 +43,18 @@ export default function TraceListRow({ trace }: { trace: Trace }) { ); } +function getResponseStatus(trace: Trace) { + const { statusCode, state } = trace.http || {}; + + if (state === HttpRequestState.Aborted) return 'Aborted'; + + return statusCode; +} + function indicatorStyle(trace: Trace) { - const statusCode = trace.http?.statusCode; + const { statusCode, state } = trace.http || {}; + + if (state === HttpRequestState.Aborted) return 'border-l-gray-500'; if (statusCode) { if (statusCode >= 500) return 'border-l-purple-500'; @@ -53,7 +65,9 @@ function indicatorStyle(trace: Trace) { } function rowStyle(trace: Trace) { - const statusCode = trace.http?.statusCode; + const { statusCode, state } = trace.http || {}; + + if (state === HttpRequestState.Aborted) return 'bg-gray-300'; if (statusCode) { if (statusCode >= 500) return 'bg-purple-200'; diff --git a/packages/webui/src/testing/mockTraces/rest.ts b/packages/webui/src/testing/mockTraces/rest.ts index 3883e9f..3b2e294 100644 --- a/packages/webui/src/testing/mockTraces/rest.ts +++ b/packages/webui/src/testing/mockTraces/rest.ts @@ -272,6 +272,31 @@ const errorEvent: Event = { }, }; +// Aborted request (no response) +const abortedEvent: Event = { + id: '99', + parentId: undefined, + serviceName: 'gql', + timestamp: elapseTime(0.4), + http: { + ...requestData('GET', 'data.restserver.com', 433, '/features'), + state: HttpRequestState.Aborted, + requestHeaders: { + 'accept': 'application/json', + 'User-Agent': ['node-fetch/1.0 (+https://github.com/bitinn/node-fetch)'], + 'accept-encoding': 'br, gzip, deflate', + }, + requestBody: undefined, + // --------- + httpVersion: '1.1', + statusCode: undefined, + statusMessage: undefined, + responseHeaders: undefined, + responseBody: undefined, + duration: 200, + }, +}; + // In flight request (no response) const inFlightEvent: Event = { id: '9', @@ -296,4 +321,13 @@ const inFlightEvent: Event = { }, }; -export default [authRequest, dataEvent, postEvent, optionsEvent, notFoundEvent, errorEvent, inFlightEvent]; +export default [ + authRequest, + dataEvent, + postEvent, + optionsEvent, + notFoundEvent, + errorEvent, + abortedEvent, + inFlightEvent, +];