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

async onRequest interceptor #439

Open
avasuro opened this issue Feb 11, 2021 · 4 comments
Open

async onRequest interceptor #439

avasuro opened this issue Feb 11, 2021 · 4 comments

Comments

@avasuro
Copy link

avasuro commented Feb 11, 2021

As far as I see from documentation there is no support for asynchronous onRequest interceptor (unlike onError interceptor, for example).

Sometimes this feature is required, e.g. if you use aws-amplify library it is common approach to get request token using Auth.currentSession() method, which is asynchronous. And because onRequest interceptor is not asynchronous I can't add token to request header without using ugly workarounds.

@klis87
Copy link
Owner

klis87 commented Feb 12, 2021

@avasuro Thx for this feature request, I already thought about this, but there is only one issue I am not sure how to solve - request aborts. For now aborts cancel request promises created by drivers, however imagine a scenario, where onRequest is running due to a pending promise, and in the middle we dispatch abort action, in theory we should cancel promise returned by onRequest interceptor, however I am not sure how easy it is to implement, also I am not sure whether just ignoring onRequest will be fine, or actually we should stop its execution somehow.

The point is, right now onRequest in synchronous, once it becomes async, it will affect request aborts. It could also affect other functionality, like mutations, polling, in theory this is super small change, in practice this is very huge.

For now you can do sth else, just create your own redux middleware or use redux thunk.

@klis87
Copy link
Owner

klis87 commented Feb 12, 2021

also, question, what if for example async thing in onRequest would fail? now there is no mechanizm to give up request in onRequest, request will be always fired

another idea, why cannot you just get auth token in separate request, for example with promiseDriver? then, store it somewhere in redux, and only then make requests which require token, could it work for you?

@avasuro
Copy link
Author

avasuro commented Feb 12, 2021

@klis87
I spent several hours yesterday to investigate this task. You are absolutely right, such small improvement brings a lot of headache and very controversial decisions.
Therefore, I implemented this functionality through my own custom driver wrapper, which can accept any type of interceptor with ability to support interceptor cancellation.
Not sure that this solution is good enough, but it works. Just in case - here is a source code, may be it will help somebody:

See source code
// createDriver.ts

import { Driver, RequestAction } from '@redux-requests/core';

interface ICreateDriverConfig {
driver: Driver;
onBeforeAjaxSend: (request: IRequest, action: RequestAction) => Promise<IRequestConfig>
}

/*
* Common driver creator function that supports
* onBeforeAjaxSend interceptor
* (any onBeforeAjaxSend function can be passed):
*/
const createDriver = ({ driver, onBeforeAjaxSend }: ICreateDriverConfig) => {
return async function sendRequest(
  requestConfig: IRequestConfig,
  requestAction: RequestAction,
  driverActions: any,
) {
  let isCancelled: boolean = false;
  let nativeRequest: any;
  let beforeAjaxSendPromise: any;
  const request = (async () => {
    if (onBeforeAjaxSend) {
      beforeAjaxSendPromise = onBeforeAjaxSend(requestConfig, requestAction);
      requestConfig = await beforeAjaxSendPromise;
    }
    if (!isCancelled) {
      nativeRequest = driver(requestConfig, requestAction, driverActions);
      return nativeRequest;
    }
  })();
  // @ts-ignore (because 'cancel' is not in Promise interface )
  request.cancel = () => {
    isCancelled = true;
    if (beforeAjaxSendPromise && beforeAjaxSendPromise.cancel) {
      beforeAjaxSendPromise.cancel();
    }
    if (nativeRequest) {
      nativeRequest.cancel();
    }
  };
  return request;
};
};

export default createDriver;
// Example of usage (somewhere in redux store init file):

import axios from 'axios';
import createDriver from './createDriver';
import { createDriver as createAxiosDriver } from '@redux-requests/axios';


const { requestsReducer, requestsMiddleware } = handleRequests({
  driver: createDriver({
      driver: createAxiosDriver(axios),
      onBeforeAjaxSend: async (request: IRequest, action: RequestAction): Promise<IRequest> => {
        if (!request.headers) {
          request.headers = {};
        }

        if (!request.headers.Authorization) {
          // Try to get current session or renew token if it expired:
          // (error is catched to make ajax call in any case, even when failed to update token):
          const session = await Auth.currentSession().catch(() => null);
          if (session) {
            const authToken = session.getIdToken().getJwtToken();
            if (authToken) {
              request.headers.Authorization = `Bearer ${authToken}`;
            }
          }
        }

        return request;
      },
    })
    // ... other handleRequests config props
})

another idea, why cannot you just get auth token in separate request, for example with promiseDriver? then, store it somewhere in redux, and only then make requests which require token, could it work for you?

Because there can be situations when token in store expired, separate request to fetch new token is made (but not finished) and new redux-requests request is started. In that case in this request old token will be used (because new token didn't received at that moment yet) which will cause request to fail, while I need to "hang" request for a while untill new token will be received and use new token in this request.

@klis87
Copy link
Owner

klis87 commented Feb 12, 2021

Nice idea with custom driver! However...

Because there can be situations when token in store expired, separate request to fetch new token is made (but not finished) and new redux-requests request is started. In that case in this request old token will be used (because new token didn't received at that moment yet) which will cause request to fail, while I need to "hang" request for a while untill new token will be received and use new token in this request.

Token expiration can be handled in onError interceptor, see full example here: https://redux-requests.klisiczynski.com/docs/tutorial/6-interceptors#metasilent-and-metarunon

The point is, you are right, a request with an expired token will fail, but that's ok! Then, you catch this error, fire a request there to get a new token, you save it, you override used token in catched request action, and you send it again. Then, from dispatched actions perspective, there won't be any error, because you catch error in onError interceptor - which you can do globally.

Another interesting idea, it could happen that multiple requests at the same time could fail due to expired token. Then, you need to be careful not to refresh token with takeLatest: true, because then it could happen that 2nd onError interceptor could abort refetching of the 1st etc. takeEvery is not optimal either, as then in theory you could make multiple requests to get a new token at the same time, while only one is enough. For such a scenario you could now check whether there is a pending token refresh and join the request with https://redux-requests.klisiczynski.com/docs/api-reference/join-request . In the future, I will introduce a new effect - takeLeading, which will do this automatically behind the scenes.

Btw, you might be interested in this discussion #397

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

No branches or pull requests

2 participants