Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REQ] Support AbortController in Typescript-Fetch #4851

Closed
rahulsom opened this issue Dec 20, 2019 · 11 comments · Fixed by #10050
Closed

[REQ] Support AbortController in Typescript-Fetch #4851

rahulsom opened this issue Dec 20, 2019 · 11 comments · Fixed by #10050

Comments

@rahulsom
Copy link

rahulsom commented Dec 20, 2019

Is your feature request related to a problem? Please describe.

The Fetch API supports attaching a signal, and using that to abort requests. This is particularly helpful when building SPAs which background a lot of requests, and the user navigates to an area of the app where some requests are no longer useful. It improves performance for the user if we can cancel pending requests so newer requests can execute sooner.

Describe the solution you'd like

Currently, this is what generated code looks like when using single param

async methodName(requestParameters: RequestType): Promise<ResponseType> {...}
interface RequestType {
  fieldName1: FieldType1;
  fieldName2: FieldType2;
}

Support either of these two methods

async methodName(requestParameters: RequestType, abortController?: AbortController): Promise<ResponseType> {...}
interface RequestType {
  fieldName1: FieldType1;
  fieldName2: FieldType2;
}

or

async methodName(requestParameters: RequestType): Promise<ResponseType> {...}
interface RequestType {
  fieldName1: FieldType1;
  fieldName2: FieldType2;
  abortController: AbortController;
}

This library seems to be commonly used for this purpose - https://github.com/mysticatea/abort-controller

Describe alternatives you've considered

I can't think of an alternative.

Additional context

None.

@rahulsom
Copy link
Author

rahulsom commented Jan 3, 2020

It looks like the Configuration has a fetchAPI field that is a function. That can be overridden to add a signal that comes from the AbortController. That is perfectly usable for this use case.

@gnuletik
Copy link

gnuletik commented Apr 15, 2020

Thanks for the tip @rahulsom!
Here's some example of implementation for anyone coming here:

// utils/api.ts
import { YourApiName, Configuration, ConfigurationParameters } from 'your-npm-package'

const config: ConfigurationParameters = {
  // your usual config...
  basePath: 'https://example.com',
  fetchApi: portableFetch // or whatever, or leave empty
}

export function newAbortableApi (abortController: AbortController): TimesideApi {
  const abortableFetch = (url: string, params: RequestInit | undefined): Promise<Response> => {
    params = params || {}
    params.signal = abortController.signal
    const configFetch = config.fetchApi || window.fetch.bind(window)
    return configFetch(url, params)
  }

  const abortConfig = new Configuration({
    ...config,
    fetchApi: abortableFetch
  })

  return new YourApiName(abortConfig)
}
// consumer.ts
import { newAbortableApi } from 'utils/api'

const abortController = new AbortController()
const api = newAbortableApi(abortController)

// make some api call
api.retrieveSomething()

// abort it after 10ms
setTimeout(() => {
  abortController.abort()
}, 10)

@RSWilli
Copy link

RSWilli commented Jul 14, 2021

@gnuletik s version works but cancels all concurrent requests at once (which may not be desired)

another way I found, if you want to have a different abort signal for each request (e.g. i you do not want to cancel all outgoing requests at once e.g. in a file upload szenario)

The implementation injects a middleware into the api and returns a clone, so it can be used multiple times with different signals

export const abortable = <T extends BaseAPI>(api: T, ctrl: AbortController): T => {
    return api.withPreMiddleware(async ctx => {
        return {
            ...ctx,
            init: {
                ...ctx.init,
                signal: ctrl.signal
            }
        }
    })
}

consumer:

const c1 = new AbortController()
const c2 = new AbortController()

abortable(yourApiInstance, c1).retrieveSomething()

abortable(yourApiInstance, c2).retrieveSomething()

// cancel only the first request
setTimeout(() => {
  c1.abort()
}, 10)

@badsyntax
Copy link
Contributor

badsyntax commented Jul 27, 2021

I'm confused by some of the suggestions in this thread.

@rahulsom are you suggesting we have to create a new instances of our apis for every request, for example?

function fetchUsers() {
  const configParams: ConfigurationParameters = {
    basePath: config.API.REST_URL,
    middleware: [new ApiMiddleware()],
    fetchApi: () => {
      // ... create AbortController instance
    }
  };

  const apiConfig = new Configuration(configParams);

  return new UsersApi(apiConfig).getUsers({});
}

function fetchUserById(userId) {
  const configParams: ConfigurationParameters = {
    basePath: config.API.REST_URL,
    middleware: [new ApiMiddleware()],
    fetchApi: () => {
       // ... create AbortController instance
    }
  };

  const apiConfig = new Configuration(configParams);

  return new UsersApi(apiConfig).getUser({id: userId});
}

A better approach is to create one instance of our API's, but then we can't update the config for each specific request.

const configParams: ConfigurationParameters = {
  basePath: config.API.REST_URL,
  middleware: [new ApiMiddleware()],
  fetchApi: () => {
    // ...
  }
};

const apiConfig = new Configuration(configParams);

const usersApi = new UsersApi(apiConfig);

function fetchUsers() {
  return usersApi.getUsers({});
}

function fetchUserById(userId) {
  return usersApi.getUser({ id: userId });
}

I would love to be able to pass fetch initParams on a per request basis:

 const controller = new AbortController();
 const signal = controller.signal;

usersApi.getUser({ id: userId }, { signal });

This way we can cancel requests without having to reconstruct our API's for every request, unless I'm missing something?

What do you think?

@badsyntax
Copy link
Contributor

These are the changes I made to the template files to allow me to pass in an initOverrides argument:

diff --git a/modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache b/modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache
index 851b9ce3831..f9dfb7c77e2 100644
--- a/modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache
+++ b/modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache
@@ -49,7 +49,7 @@ export interface {{classname}}Interface {
      * @throws {RequiredError}
      * @memberof {{classname}}Interface
      */
-    {{nickname}}Raw({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request{{/allParams.0}}): Promise<runtime.ApiResponse<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>>;
+    {{nickname}}Raw({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request, {{/allParams.0}}initOverrides?: RequestInit): Promise<runtime.ApiResponse<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>>;

     /**
      {{#notes}}
@@ -60,10 +60,10 @@ export interface {{classname}}Interface {
      {{/summary}}
      */
     {{^useSingleRequestParameter}}
-    {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{#isNullable}} | null{{/isNullable}}{{/isEnum}}{{^-last}}, {{/-last}}{{/allParams}}): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>;
+    {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{#isNullable}} | null{{/isNullable}}{{/isEnum}}{{^-last}}, {{/-last}}, {{/allParams}}initOverrides?: RequestInit): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>;
     {{/useSingleRequestParameter}}
     {{#useSingleRequestParameter}}
-    {{nickname}}({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request{{/allParams.0}}): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>;
+    {{nickname}}({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request, {{/allParams.0}}initOverrides?: RequestInit): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>;
     {{/useSingleRequestParameter}}

 {{/operation}}
@@ -91,7 +91,7 @@ export class {{classname}} extends runtime.BaseAPI {
      * {{&summary}}
      {{/summary}}
      */
-    async {{nickname}}Raw({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request{{/allParams.0}}): Promise<runtime.ApiResponse<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>> {
+    async {{nickname}}Raw({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request, {{/allParams.0}}initOverrides?: RequestInit): Promise<runtime.ApiResponse<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>> {
         {{#allParams}}
         {{#required}}
         if (requestParameters.{{paramName}} === null || requestParameters.{{paramName}} === undefined) {
@@ -284,7 +284,7 @@ export class {{classname}} extends runtime.BaseAPI {
             {{#hasFormParams}}
             body: formParams,
             {{/hasFormParams}}
-        });
+        }, initOverrides);

         {{#returnType}}
         {{#isResponseFile}}
@@ -331,24 +331,24 @@ export class {{classname}} extends runtime.BaseAPI {
      {{/summary}}
      */
     {{^useSingleRequestParameter}}
-    async {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{#isNullable}} | null{{/isNullable}}{{/isEnum}}{{^-last}}, {{/-last}}{{/allParams}}): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> {
+    async {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{#isNullable}} | null{{/isNullable}}{{/isEnum}}{{^-last}}, {{/-last}}, {{/allParams}}initOverrides?: RequestInit): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> {
         {{#returnType}}
-        const response = await this.{{nickname}}Raw({{#allParams.0}}{ {{#allParams}}{{paramName}}: {{paramName}}{{^-last}}, {{/-last}}{{/allParams}} }{{/allParams.0}});
+        const response = await this.{{nickname}}Raw({{#allParams.0}}{ {{#allParams}}{{paramName}}: {{paramName}}{{^-last}}, {{/-last}}{{/allParams}} }, {{/allParams.0}}initOverrides);
         return await response.value();
         {{/returnType}}
         {{^returnType}}
-        await this.{{nickname}}Raw({{#allParams.0}}{ {{#allParams}}{{paramName}}: {{paramName}}{{^-last}}, {{/-last}}{{/allParams}} }{{/allParams.0}});
+        await this.{{nickname}}Raw({{#allParams.0}}{ {{#allParams}}{{paramName}}: {{paramName}}{{^-last}}, {{/-last}}{{/allParams}} }{{/allParams.0}}, initOverrides);
         {{/returnType}}
     }
     {{/useSingleRequestParameter}}
     {{#useSingleRequestParameter}}
-    async {{nickname}}({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request{{/allParams.0}}): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> {
+    async {{nickname}}({{#allParams.0}}requestParameters: {{#prefixParameterInterfaces}}{{classname}}{{/prefixParameterInterfaces}}{{operationIdCamelCase}}Request, {{/allParams.0}}initOverrides?: RequestInit): Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> {
         {{#returnType}}
-        const response = await this.{{nickname}}Raw({{#allParams.0}}requestParameters{{/allParams.0}});
+        const response = await this.{{nickname}}Raw({{#allParams.0}}requestParameters, {{/allParams.0}}initOverrides);
         return await response.value();
         {{/returnType}}
         {{^returnType}}
-        await this.{{nickname}}Raw({{#allParams.0}}requestParameters{{/allParams.0}});
+        await this.{{nickname}}Raw({{#allParams.0}}requestParameters, {{/allParams.0}}initOverrides);
         {{/returnType}}
     }
     {{/useSingleRequestParameter}}
diff --git a/modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache b/modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache
index 8df16e9074e..baae758566f 100644
--- a/modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache
+++ b/modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache
@@ -33,8 +33,8 @@ export class BaseAPI {
         return this.withMiddleware<T>(...middlewares);
     }

-    protected async request(context: RequestOpts): Promise<Response> {
-        const { url, init } = this.createFetchParams(context);
+    protected async request(context: RequestOpts, initOverrides?: RequestInit): Promise<Response> {
+        const { url, init } = this.createFetchParams(context, initOverrides);
         const response = await this.fetchApi(url, init);
         if (response.status >= 200 && response.status < 300) {
             return response;
@@ -42,7 +42,7 @@ export class BaseAPI {
         throw response;
     }

-    private createFetchParams(context: RequestOpts) {
+    private createFetchParams(context: RequestOpts, initOverrides?: RequestInit) {
         let url = this.configuration.basePath + context.path;
         if (context.query !== undefined && Object.keys(context.query).length !== 0) {
             // only add the querystring to the URL if there are query parameters.
@@ -59,7 +59,8 @@ export class BaseAPI {
             method: context.method,
             headers: headers,
             body,
-            credentials: this.configuration.credentials
+            credentials: this.configuration.credentials,
+            ...initOverrides
         };
         return { url, init };
     }

Now I can pass an AbortSignal on a per-request basis:

usersApi.getUser({ id: userId }, { signal });

@rahulsom
Copy link
Author

@badsyntax
This is what I ended up doing.

const configParams: ConfigurationParameters = {
  basePath: config.API.REST_URL,
  middleware: [new ApiMiddleware()],
  fetchApi: () => {
    // ...
  }
};

const apiConfig = new Configuration(configParams);

const usersApi = new UsersApi(apiConfig);

function fetchUsers() {
  return usersApi.getUsers({});
}

function fetchUserById(userId) {
  return usersApi.getUser({ id: userId });
}

I like what you're doing with

const controller = new AbortController();
const signal = controller.signal;

usersApi.getUser({ id: userId }, { signal });

@RSWilli
Copy link

RSWilli commented Jul 28, 2021

@badsyntax that change seems like something that could be useful to others (also for all other options that can be passed to fetch)

maybe create a PR?

@badsyntax
Copy link
Contributor

@RSWilli ok i will try send a PR today

@badsyntax
Copy link
Contributor

I created a PR here: #10050

@badsyntax
Copy link
Contributor

Btw you can now pass in initOverrides for each request, which supports passing in an AbortController signal. That change is released with v5.2.1.

@sascha-krug
Copy link

Hi, there, I wonder where to set the timeout value it self? Can't find any examples anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants