Skip to content

Commit

Permalink
Handle aborted requests (#158)
Browse files Browse the repository at this point in the history
Co-authored-by: Kevin Paxton <[email protected]>
  • Loading branch information
KenanYusuf and kgpax authored Nov 6, 2023
1 parent aa050d2 commit 9b61037
Show file tree
Hide file tree
Showing 10 changed files with 194 additions and 12 deletions.
8 changes: 8 additions & 0 deletions .changeset/spotty-hats-learn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@envyjs/webui': minor
'@envyjs/core': minor
'@envyjs/node': minor
'@envyjs/web': minor
---

Handle aborted requests
26 changes: 25 additions & 1 deletion packages/core/src/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
getEventFromAbortedFetchRequest,
getEventFromFetchRequest,
getEventFromFetchResponse,
getUrlFromFetchRequest,
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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');
Expand Down
15 changes: 14 additions & 1 deletion packages/core/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Event> {
return {
...req,

http: {
...req.http!,
state: HttpRequestState.Received,
Expand Down
24 changes: 22 additions & 2 deletions packages/node/src/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { Plugin, getEventFromFetchRequest, getEventFromFetchResponse } from '@envyjs/core';
import {
Plugin,
getEventFromAbortedFetchRequest,
getEventFromFetchRequest,
getEventFromFetchResponse,
} from '@envyjs/core';

import { generateId } from './id';

Expand All @@ -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);

Expand Down
25 changes: 23 additions & 2 deletions packages/web/src/fetch.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -8,15 +13,31 @@ 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);
exporter.send(reqEvent);

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);

Expand Down
24 changes: 24 additions & 0 deletions packages/webui/src/components/ui/TraceDetail.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<TraceDetail />);

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(<TraceDetail />);

const summary = getByTestId('summary');
const abortedIndicator = within(summary).queryByTestId('aborted-indicator');

expect(abortedIndicator).not.toBeInTheDocument();
});

it('should display full URL', () => {
getSelectedTraceFn.mockReturnValue({
...mockTrace,
Expand Down
7 changes: 6 additions & 1 deletion packages/webui/src/components/ui/TraceDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -102,6 +102,11 @@ export default function TraceDetail() {
<img src={getIconUri(trace)} alt="" className="w-7 h-7" />
</div>
<Badge>{method}</Badge>
{requestAborted && (
<Badge className="bg-gray-500 uppercase" data-test-id="aborted-indicator">
Aborted
</Badge>
)}
{statusCode && (
<Badge className={statusCodeStyle(statusCode)} data-test-id="method">
{httpStatusLabel}
Expand Down
19 changes: 19 additions & 0 deletions packages/webui/src/components/ui/TraceListRow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<TraceListRow trace={trace} />);
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',
Expand Down
22 changes: 18 additions & 4 deletions packages/webui/src/components/ui/TraceListRow.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { HttpRequestState } from '@envyjs/core';

import { Loading } from '@/components';
import useApplication from '@/hooks/useApplication';
import { ListDataComponent } from '@/systems';
Expand Down Expand Up @@ -27,8 +29,8 @@ export default function TraceListRow({ trace }: { trace: Trace }) {
<div className="font-semibold" data-test-id="column-data-method-cell">
{trace.http?.method.toUpperCase()}
</div>
<div className="font-semibold text-xs text-opacity-70" data-test-id="column-data-code-cell">
{trace.http?.statusCode}
<div className="font-semibold text-xs text-opacity-70 uppercase" data-test-id="column-data-code-cell">
{getResponseStatus(trace)}
</div>
</TraceListRowCell>
<TraceListRowCell data-test-id="column-data-request-cell">
Expand All @@ -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';
Expand All @@ -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';
Expand Down
36 changes: 35 additions & 1 deletion packages/webui/src/testing/mockTraces/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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,
];

0 comments on commit 9b61037

Please sign in to comment.