fix: Fix MastoTimeoutError to be thrown instead of AbortError

neet · Dec 24, 2022 · 8de20a4 · 8de20a4
1 parent 78fe285
commit 8de20a4
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 56 deletions.
diff --git a/src/config.spec.ts b/src/config.spec.ts
@@ -180,7 +180,7 @@ describe('Config', () => {
       new SerializerNativeImpl(),
     );
 
-    const signal = config.createAbortController();
+    const signal = config.createAbortSignal();
 
     const callback = jest.fn();
     signal.addEventListener('abort', callback);

diff --git a/src/config.ts b/src/config.ts
@@ -33,10 +33,6 @@ export class MastoConfig {
     private readonly serializer: Serializer,
   ) {}
 
-  get timeout(): number {
-    return this.props.timeout ?? DEFAULT_TIMEOUT_MS;
-  }
-
   createHeader(override: HeadersInit = {}): Headers {
     const headersInit = mergeHeadersInit([
       this.props.defaultRequestInit?.headers ?? {},
@@ -83,21 +79,27 @@ export class MastoConfig {
     return url.toString();
   }
 
-  createAbortController(signal?: AbortSignal | null): AbortSignal {
+  createTimeoutSignal(): AbortSignal {
     const timeoutController = new AbortController();
 
-    const signals: AbortSignal[] = [timeoutController.signal];
-    if (signal != undefined) {
-      signals.push(signal);
-    }
+    setTimeout(() => {
+      timeoutController.abort();
+    }, this.props.timeout ?? DEFAULT_TIMEOUT_MS);
+
+    return timeoutController.signal;
+  }
+
+  createAbortSignal(signal?: AbortSignal | null): AbortSignal {
+    const signals: AbortSignal[] = [this.createTimeoutSignal()];
+
     if (this.props.defaultRequestInit?.signal) {
       // FIXME: `abort-controller` and `node-fetch` mismatches
       signals.push(this.props.defaultRequestInit.signal as AbortSignal);
     }
 
-    setTimeout(() => {
-      timeoutController.abort();
-    }, this.timeout);
+    if (signal != undefined) {
+      signals.push(signal);
+    }
 
     return mergeAbortSignals(signals);
   }

diff --git a/src/http/http-native-impl.spec.ts b/src/http/http-native-impl.spec.ts
@@ -0,0 +1,21 @@
+import { MastoConfig } from '../config';
+import { MastoTimeoutError } from '../errors';
+import { SerializerNativeImpl } from '../serializers';
+import { HttpNativeImpl } from './http-native-impl';
+
+it('timeouts', async () => {
+  const serializer = new SerializerNativeImpl();
+  const http = new HttpNativeImpl(
+    serializer,
+    new MastoConfig(
+      {
+        url: 'https://example.com',
+        streamingApiUrl: 'wss://example.com',
+        timeout: 0,
+      },
+      serializer,
+    ),
+  );
+
+  await expect(() => http.get('/')).rejects.toThrowError(MastoTimeoutError);
+});
diff --git a/src/http/http-native-impl.ts b/src/http/http-native-impl.ts
@@ -3,7 +3,11 @@ import { fetch, FormData, Request, Response } from '@mastojs/ponyfills';
 
 import type { MastoConfig } from '../config';
 import type { CreateErrorParams } from '../errors';
-import { createHttpError, MastoUnexpectedError } from '../errors';
+import {
+  createHttpError,
+  MastoTimeoutError,
+  MastoUnexpectedError,
+} from '../errors';
 import type { Logger } from '../logger';
 import type { Serializer } from '../serializers';
 import { BaseHttp } from './base-http';
@@ -53,7 +57,7 @@ export class HttpNativeImpl extends BaseHttp implements Http {
 
     const url = this.config.resolveHttpPath(path, searchParams);
     const headers = this.config.createHeader(requestInit?.headers);
-    const abortSignal = this.config.createAbortController(
+    const abortSignal = this.config.createAbortSignal(
       requestInit?.signal as AbortSignal,
     );
     const body = this.serializer.serialize(
@@ -77,24 +81,30 @@ export class HttpNativeImpl extends BaseHttp implements Http {
   }
 
   private async createError(error: unknown): Promise<unknown> {
-    if (!(error instanceof Response)) {
-      return error;
+    if (error instanceof Response) {
+      const data = this.serializer.deserialize(
+        getContentType(error.headers) ?? 'application/json',
+        await error.text(),
+      );
+
+      return createHttpError({
+        cause: error,
+        statusCode: error.status,
+        message: data?.error,
+        details: data?.errorDescription,
+        description: data?.details,
+        limit: error.headers.get('X-RateLimit-Limit'),
+        remaining: error.headers.get('X-RateLimit-Remaining'),
+        reset: error.headers.get('X-RateLimit-Reset'),
+      } as CreateErrorParams);
     }
 
-    const data = this.serializer.deserialize(
-      getContentType(error.headers) ?? 'application/json',
-      await error.text(),
-    );
+    // TODO: Use abort reason
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    if (error != undefined && (error as any).name === 'AbortError') {
+      return new MastoTimeoutError(`Request timed out`, { cause: error });
+    }
 
-    return createHttpError({
-      cause: error,
-      statusCode: error.status,
-      message: data?.error,
-      details: data?.errorDescription,
-      description: data?.details,
-      limit: error.headers.get('X-RateLimit-Limit'),
-      remaining: error.headers.get('X-RateLimit-Remaining'),
-      reset: error.headers.get('X-RateLimit-Reset'),
-    } as CreateErrorParams);
+    return error;
   }
 }
diff --git a/src/mastodon/v2/repositories/media-attachment-repository.ts b/src/mastodon/v2/repositories/media-attachment-repository.ts
@@ -1,9 +1,9 @@
 import type { MastoConfig } from '../../../config';
 import { version } from '../../../decorators';
-import { MastoHttpNotFoundError } from '../../../errors';
+import { MastoHttpNotFoundError, MastoTimeoutError } from '../../../errors';
 import type { Http } from '../../../http';
 import type { Logger } from '../../../logger';
-import { delay, timeout } from '../../../utils';
+import { delay } from '../../../utils';
 import type { MediaAttachment } from '../../v1';
 import { MediaAttachmentRepository as MediaAttachmentRepositoryV1 } from '../../v1';
 
@@ -42,32 +42,35 @@ export class MediaAttachmentRepository {
    * @param interval interval of polling
    * @returns Media attachment that has done processing
    */
-  waitFor(id: string, interval = 1000): Promise<MediaAttachment> {
-    return timeout(
-      (async () => {
-        let media: MediaAttachment | undefined;
+  async waitFor(id: string, interval = 1000): Promise<MediaAttachment> {
+    const signal = this.config.createTimeoutSignal();
+    let media: MediaAttachment | undefined;
 
-        while (media == undefined) {
-          await delay(interval);
-          try {
-            const processing = await this.v1.fetch(id);
+    while (media == undefined) {
+      if (signal.aborted) {
+        throw new MastoTimeoutError(
+          'The media encoding has been timed out in your instance.',
+        );
+      }
 
-            if (processing.url != undefined) {
-              media = processing;
-            }
-          } catch (error) {
-            // Some instance caches API response
-            if (error instanceof MastoHttpNotFoundError) {
-              continue;
-            }
-            throw error;
-          }
+      await delay(interval);
+
+      try {
+        const processing = await this.v1.fetch(id);
+
+        if (processing.url != undefined) {
+          media = processing;
+        }
+      } catch (error) {
+        // Some instance caches API response
+        if (error instanceof MastoHttpNotFoundError) {
+          continue;
         }
+        throw error;
+      }
+    }
 
-        return media;
-      })(),
-      this.config?.timeout,
-    );
+    return media;
   }
 
   /**

diff --git a/src/utils/merge-abort-signals.spec.ts b/src/utils/merge-abort-signals.spec.ts
@@ -10,7 +10,7 @@ describe('mergeAbortSignals', () => {
     const b = new AbortController();
 
     const merged = mergeAbortSignals([a.signal, b.signal]);
-    const oneOfAB = [a, b][getRandomInt() % 2 === 0 ? 0 : 1];
+    const oneOfAB = [a, b][getRandomInt() % 2];
     oneOfAB.abort();
 
     expect(merged?.aborted).toBe(true);