Skip to content

Commit

Permalink
Merge pull request #236 from axiomhq/gabrielelpidio/fetch-timeouts
Browse files Browse the repository at this point in the history
fix: add timeout to fetch
  • Loading branch information
gabrielelpidio authored Nov 19, 2024
2 parents 1fbaab3 + c0feb60 commit e500b03
Show file tree
Hide file tree
Showing 7 changed files with 379 additions and 12 deletions.
3 changes: 3 additions & 0 deletions packages/js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,8 @@
"import": "./dist/esm/index.js",
"require": "./dist/cjs/index.cjs",
"default": "./dist/esm/index.js"
},
"devDependencies": {
"msw": "^2.6.2"
}
}
11 changes: 9 additions & 2 deletions packages/js/src/batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,15 @@ export class Batch {
return;
}

const res = await this.ingestFn(this.id, events, this.options);
this.lastFlush = new Date();
let res = null;
try {
res = await this.ingestFn(this.id, events, this.options);
} catch (e) {
throw e;
} finally {
this.lastFlush = new Date();
}

return res;
};
}
2 changes: 2 additions & 0 deletions packages/js/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class BaseClient extends HTTPClient {
'streaming-duration': options?.streamingDuration as string,
nocache: options?.noCache as boolean,
},
120_000,
);

/**
Expand Down Expand Up @@ -129,6 +130,7 @@ class BaseClient extends HTTPClient {
nocache: options?.noCache as boolean,
format: options?.format ?? 'legacy',
},
120_000,
)
.then((res) => {
if (options?.format !== 'tabular') {
Expand Down
23 changes: 14 additions & 9 deletions packages/js/src/fetchClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export class FetchClient {
method: string,
init: RequestInit = {},
searchParams: { [key: string]: string } = {},
timeout = this.config.timeout,
): Promise<T> {
let finalUrl = `${this.config.baseUrl}${endpoint}`;
const params = this._prepareSearchParams(searchParams);
Expand All @@ -27,6 +28,7 @@ export class FetchClient {
headers,
method,
body: init.body ? init.body : undefined,
signal: AbortSignal.timeout(timeout),
cache: 'no-cache',
});

Expand All @@ -46,20 +48,20 @@ export class FetchClient {
return (await resp.json()) as T;
}

post<T>(url: string, init: RequestInit = {}, searchParams: any = {}): Promise<T> {
return this.doReq<T>(url, 'POST', init, searchParams);
post<T>(url: string, init: RequestInit = {}, searchParams: any = {}, timeout = this.config.timeout): Promise<T> {
return this.doReq<T>(url, 'POST', init, searchParams, timeout);
}

get<T>(url: string, init: RequestInit = {}, searchParams: any = {}): Promise<T> {
return this.doReq<T>(url, 'GET', init, searchParams);
get<T>(url: string, init: RequestInit = {}, searchParams: any = {}, timeout = this.config.timeout): Promise<T> {
return this.doReq<T>(url, 'GET', init, searchParams, timeout);
}

put<T>(url: string, init: RequestInit = {}, searchParams: any = {}): Promise<T> {
return this.doReq<T>(url, 'PUT', init, searchParams);
put<T>(url: string, init: RequestInit = {}, searchParams: any = {}, timeout = this.config.timeout): Promise<T> {
return this.doReq<T>(url, 'PUT', init, searchParams, timeout);
}

delete<T>(url: string, init: RequestInit = {}, searchParams: any = {}): Promise<T> {
return this.doReq<T>(url, 'DELETE', init, searchParams);
delete<T>(url: string, init: RequestInit = {}, searchParams: any = {}, timeout = this.config.timeout): Promise<T> {
return this.doReq<T>(url, 'DELETE', init, searchParams, timeout);
}

_prepareSearchParams = (searchParams: { [key: string]: string }) => {
Expand All @@ -80,7 +82,10 @@ export class FetchClient {
export class AxiomTooManyRequestsError extends Error {
public message: string = '';

constructor(public limit: Limit, public shortcircuit = false) {
constructor(
public limit: Limit,
public shortcircuit = false,
) {
super();
Object.setPrototypeOf(this, AxiomTooManyRequestsError.prototype); // https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work
const retryIn = AxiomTooManyRequestsError.timeUntilReset(limit);
Expand Down
2 changes: 1 addition & 1 deletion packages/js/src/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default abstract class HTTPClient {
this.client = new FetchClient({
baseUrl,
headers,
timeout: 3000,
timeout: 20_000,
});
}
}
52 changes: 52 additions & 0 deletions packages/js/test/unit/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { describe, expect, it, beforeEach, vi } from 'vitest';
import { setupServer } from 'msw/node';
import { http, HttpResponse } from 'msw';

import { ContentType, ContentEncoding, Axiom, AxiomWithoutBatching } from '../../src/client';
import { AxiomTooManyRequestsError } from '../../src/fetchClient';
Expand Down Expand Up @@ -320,6 +322,35 @@ describe('Axiom', () => {

expect(true).toEqual(errorCaptured);
}, 50000);

it('times out after 20s', async () => {
// Can't use fake timers because they don't support AbortSignals https://github.com/sinonjs/fake-timers/issues/418
const server = setupServer(
http.post(`${clientURL}/*`, async () => {
await new Promise((resolve) => setTimeout(resolve, 21_000));
return HttpResponse.json();
}),
);

server.listen();

let error: string | null = null;

let client = new AxiomWithoutBatching({
url: clientURL,
token: 'test',
onError: (err) => {
console.error('error callback has been called', err);
error = err.name;
},
});

await client.ingest('test', [{ name: 'test' }]);

expect(error).toEqual('TimeoutError');

server.close();
}, 21_500);
});

describe('Querying', async () => {
Expand Down Expand Up @@ -392,6 +423,27 @@ describe('Axiom', () => {
expect(response).not.toEqual('undefined');
expect(response.tables).toHaveLength(1);
});

it('times out query after 120s', async () => {
// Can't use fake timers because they don't support AbortSignals https://github.com/sinonjs/fake-timers/issues/418
const server = setupServer(
http.post(`${clientURL}/*`, async () => {
await new Promise((resolve) => setTimeout(resolve, 121_000));
return HttpResponse.json();
}),
);

server.listen();

let client = new AxiomWithoutBatching({
url: clientURL,
token: 'test',
});

await expect(client.query("['test'] | count")).rejects.toThrow('The operation was aborted due to timeout');

server.close();
}, 125_000);
});

describe('Tokens', () => {
Expand Down
Loading

0 comments on commit e500b03

Please sign in to comment.