Skip to content

Commit

Permalink
fix: Clear timeout when request completed
Browse files Browse the repository at this point in the history
  • Loading branch information
neet committed Dec 25, 2022
1 parent 34f95a6 commit f6b2f9c
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 64 deletions.
25 changes: 0 additions & 25 deletions src/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,29 +166,4 @@ describe('Config', () => {

expect(config.createWebsocketProtocols()).toEqual([]);
});

it('creates timeout controller with specific limit', () => {
jest.useFakeTimers();
const config = new MastoConfig(
{
url: 'https://mastodon.social',
streamingApiUrl: 'wss://mastodon.social',
version: new SemVer('0.0.1'),
accessToken: 'token',
timeout: 3000,
},
new SerializerNativeImpl(),
);

const signal = config.createAbortSignal();

const callback = jest.fn();
signal.addEventListener('abort', callback);

jest.advanceTimersByTime(2900);
expect(callback).not.toBeCalled();
jest.advanceTimersByTime(100);
expect(callback).toBeCalled();
jest.clearAllTimers();
});
});
21 changes: 8 additions & 13 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type { AbortSignal, HeadersInit, RequestInit } from '@mastojs/ponyfills';
import { AbortController, Headers } from '@mastojs/ponyfills';
import { Headers } from '@mastojs/ponyfills';
import { gt, gte, lt, SemVer } from 'semver';

import type { LogType } from './logger';
import { LogLevel } from './logger';
import type { Serializer } from './serializers';
import { mergeAbortSignals, mergeHeadersInit } from './utils';
import { mergeAbortSignals, mergeHeadersInit, Timeout } from './utils';

const DEFAULT_TIMEOUT_MS = 1000 * 300;

Expand Down Expand Up @@ -79,18 +79,13 @@ export class MastoConfig {
return url.toString();
}

createTimeoutSignal(): AbortSignal {
const timeoutController = new AbortController();

setTimeout(() => {
timeoutController.abort();
}, this.props.timeout ?? DEFAULT_TIMEOUT_MS);

return timeoutController.signal;
createTimeout(): Timeout {
return new Timeout(this.props.timeout ?? DEFAULT_TIMEOUT_MS);
}

createAbortSignal(signal?: AbortSignal | null): AbortSignal {
const signals: AbortSignal[] = [this.createTimeoutSignal()];
createAbortSignal(signal?: AbortSignal | null): [AbortSignal, Timeout] {
const timeout = this.createTimeout();
const signals: AbortSignal[] = [timeout.signal];

if (this.props.defaultRequestInit?.signal) {
// FIXME: `abort-controller` and `node-fetch` mismatches
Expand All @@ -101,7 +96,7 @@ export class MastoConfig {
signals.push(signal);
}

return mergeAbortSignals(signals);
return [mergeAbortSignals(signals), timeout];
}

getLogLevel(): LogLevel {
Expand Down
12 changes: 8 additions & 4 deletions src/http/http-native-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '../errors';
import type { Logger } from '../logger';
import type { Serializer } from '../serializers';
import type { Timeout } from '../utils';
import { BaseHttp } from './base-http';
import { getContentType } from './get-content-type';
import type { Http, HttpRequestParams, HttpRequestResult } from './http';
Expand All @@ -24,7 +25,7 @@ export class HttpNativeImpl extends BaseHttp implements Http {
}

async request(params: HttpRequestParams): Promise<HttpRequestResult> {
const request = this.createRequest(params);
const [request, timeout] = this.createRequest(params);

try {
this.logger?.debug(`↑ ${request.method} ${request.url}`, request.body);
Expand All @@ -33,6 +34,7 @@ export class HttpNativeImpl extends BaseHttp implements Http {
throw response;
}

timeout.clear();
const text = await response.text();
const contentType = getContentType(response.headers);
if (contentType == undefined) {
Expand All @@ -52,12 +54,12 @@ export class HttpNativeImpl extends BaseHttp implements Http {
}
}

private createRequest(params: HttpRequestParams): Request {
private createRequest(params: HttpRequestParams): [Request, Timeout] {
const { path, searchParams, requestInit } = params;

const url = this.config.resolveHttpPath(path, searchParams);
const headers = this.config.createHeader(requestInit?.headers);
const abortSignal = this.config.createAbortSignal(
const [abortSignal, timeout] = this.config.createAbortSignal(
requestInit?.signal as AbortSignal,
);
const body = this.serializer.serialize(
Expand All @@ -72,12 +74,14 @@ export class HttpNativeImpl extends BaseHttp implements Http {
headers.delete('Content-Type');
}

return new Request(url, {
const request = new Request(url, {
...requestInit,
headers,
body,
signal: abortSignal,
});

return [request, timeout];
}

private async createError(error: unknown): Promise<unknown> {
Expand Down
5 changes: 3 additions & 2 deletions src/mastodon/v2/repositories/media-attachment-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ export class MediaAttachmentRepository {
* @returns Media attachment that has done processing
*/
async waitFor(id: string, interval = 1000): Promise<MediaAttachment> {
const signal = this.config.createTimeoutSignal();
const timeout = this.config.createTimeout();
let media: MediaAttachment | undefined;

while (media == undefined) {
if (signal.aborted) {
if (timeout.signal.aborted) {
throw new MastoTimeoutError(
'The media encoding has been timed out in your instance.',
);
Expand All @@ -60,6 +60,7 @@ export class MediaAttachmentRepository {

if (processing.url != undefined) {
media = processing;
timeout.clear();
}
} catch (error) {
// Some instance caches API response
Expand Down
17 changes: 17 additions & 0 deletions src/utils/timeout.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Timeout } from './timeout';

it('creates timeout controller with specific limit', () => {
jest.useFakeTimers();
const timeout = new Timeout(1000 * 3);

const callback = jest.fn();
timeout.signal.addEventListener('abort', callback);

jest.advanceTimersByTime(2900);
expect(callback).not.toBeCalled();
jest.advanceTimersByTime(100);
expect(callback).toBeCalled();
jest.clearAllTimers();

timeout.clear();
});
37 changes: 17 additions & 20 deletions src/utils/timeout.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
import { MastoTimeoutError } from '../errors';
import type { AbortSignal } from '@mastojs/ponyfills';
import { AbortController } from '@mastojs/ponyfills';

export const timeout = async <T>(task: Promise<T>, ms?: number): Promise<T> => {
// It actually is depending on the runtime...
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let cancellationToken: any | undefined;
export class Timeout {
private readonly abortController: AbortController;
private readonly timeout: NodeJS.Timeout;

if (ms == undefined) {
return task;
constructor(millisecond: number) {
this.abortController = new AbortController();
this.timeout = setTimeout(() => {
this.abortController.abort();
}, millisecond);
}

const timeoutPromise = new Promise<never>((_, reject) => {
cancellationToken = setTimeout(
() => void reject(new MastoTimeoutError(`Timeout of ${ms}ms exceeded`)),
ms,
) as unknown as number;
});

const mainPromise = task.then((value) => {
clearTimeout(cancellationToken as NodeJS.Timeout);
return value;
});
get signal(): AbortSignal {
return this.abortController.signal;
}

return Promise.race([timeoutPromise, mainPromise]);
};
clear(): void {
clearTimeout(this.timeout);
}
}

0 comments on commit f6b2f9c

Please sign in to comment.