connor4312 / cockatiel

🐦 A resilience and fault-handling library. Supports Backoffs, Retries, Circuit Breakers, Timeouts, Bulkhead Isolation, and Fallbacks.
MIT License
1.59k stars 50 forks source link

Incompatible types for AbortSignal #59

Closed HonzaMac closed 2 years ago

HonzaMac commented 2 years ago

With recent update of typescript to 4.6.4 -> 4.7.2 (in out repo), types for AbortController stopped to be compatible.

Types in node-fetch vs. cockatiel types. We use v3.0.0-beta.0.

We got this error on our side:

$ tsc -b tsconfig.build.json
../common/utils/fetch/fetchRetry.ts(52,9): error TS2322: Type 'import("/usr/app/node_modules/@types/node-fetch/externals").AbortSignal | undefined' is not assignable to type 'AbortSignal | undefined'.
  Type 'AbortSignal' is missing the following properties from type 'AbortSignal': reason, throwIfAborted
../common/clients/client.ts(105,9): error TS2322: Type 'AbortSignal | undefined' is not assignable to type 'AbortSignal | null | undefined'.
  Type 'AbortSignal' is not assignable to type 'import("/usr/app/node_modules/@types/node-fetch/externals").AbortSignal'.
    Types of property 'onabort' are incompatible.
      Type '((this: AbortSignal, ev: Event) => any) | null' is not assignable to type '((this: AbortSignal, event: any) => void) | null'.
        Type '(this: AbortSignal, ev: Event) => any' is not assignable to type '(this: AbortSignal, event: any) => void'.
          The 'this' types of each signature are incompatible.
            Type 'import("/usr/app/node_modules/@types/node-fetch/externals").AbortSignal' is not assignable to type 'AbortSignal'.
../common/utils/fetch/fetchRetry.test.ts(75,60): error TS2322: Type 'AbortSignal' is not assignable to type 'import("/usr/app/node_modules/@types/node-fetch/externals").AbortSignal'.
error Command failed with exit code 1.

Where fetchRetry looks like this:

import fetch, {RequestInfo, RequestInit, Response} from 'node-fetch';
import {Policy} from 'cockatiel';
...
export const fetchRetry = (url: RequestInfo, init: RetryRequestInit): Promise<Response> => {
  const fnFetch = () => fetch(url, init);
  const signal: AbortSignal | undefined = init?.signal ?? undefined;

  return isIdempotentHttpMethod(init.method)
    ? retryPolicyForIdempotentHttpMethods.execute(fnFetch, signal)
    : retryPolicyForNonIdempotentHttpMethods.execute(fnFetch, signal);
};
}
connor4312 commented 2 years ago

Ugh, AbortSignal is a mess right now. TypeScript has a stub of the interface in its webworker types, which is incompatible with the types the @types/node provides (that in turn is missing event listener types), which is also not exactly what node-fetch uses...

connor4312 commented 2 years ago

I made a change that should make this work. We'll just depend on there being a global AbortSignal (either via DOM/WebWorker or Node types) and I think everything should work out. I'll publish later today.