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

bug: Request doesn't execute after a abortion. #317

Closed
SamueleCaprioli97 opened this issue Aug 13, 2022 · 7 comments · Fixed by #368
Closed

bug: Request doesn't execute after a abortion. #317

SamueleCaprioli97 opened this issue Aug 13, 2022 · 7 comments · Fixed by #368
Assignees
Labels
axios related Issues that are more related to axios than to axios-cache-interceptor.

Comments

@SamueleCaprioli97
Copy link

Describe the bug

I've experienced that when i try to cancel a request and resubmit it right after, nothing seems to happen.

To Reproduce

In this CodePen, if i click the 'Request' button as first, everything seems to be fine, but after a reload, if i click first the 'With Quick Abort', all further requests will not be initiated.

Expected behavior

After the page load, when 'With Quick Abort' button is clicked, the first request will be aborted, while the second one will be sent and completed.

Additional context

  • Axios: v0.27.2
  • Axios Cache Interceptor: v0.10.7
  • What storage is being used: MemoryStorage
  • Node Version: v16.14.0
  • Browser (if applicable): Chrome 104.0.5112.81

P.S.: This plugin is very powerful and easy to use, I hope you can help me or tell me if I'm doing something wrong! 😊

@SamueleCaprioli97 SamueleCaprioli97 added the bug Something isn't working label Aug 13, 2022
@arthurfiorette
Copy link
Owner

Hey, thanks for your bug report.

I can't look into it this weekend, I'll try to fix it somewhere between Monday and next weekend...

@arthurfiorette
Copy link
Owner

arthurfiorette commented Aug 18, 2022

Hey @SamueleCaprioli97, i have some bad news (for now)...

Looking closely, this was the same error I got when building an axios cache interceptor react hooks library (axios-cache-hooks).

Let me explain it:

When a request throws any error, the response interceptor executes the onRejected response interceptor function, instead of the onResolved:

const onRejected: ResponseInterceptor['onRejected'] = async (error) => {
const config = error.config as CacheRequestConfig;

And these functions are stateless, so the only thing that guides us on each request it the provided error parameter. The problem is, when the error is from a cancelled request, axios does not returns any other information.

(this is everything available at line 204)
image

Coming back to axios-cache-hooks, I've detected it some time ago with the automatically cancellation mechanism and pushed a PR to axios that fixed it, axios/axios#4659.

The problem is that v0.27.2 seems to be our last version before 1.X, that's why it never actually got released into a new patch version (and why axios-cache-hooks is frozen in the time).

Our current hope is to get jasonsaayman to cherry pick this PR into a v0 patch release. When the patch gets released, automagically this problem will be fixed, so, no need to a new axios-cache-interceptor version.

P.S.: Thanks for helping this package :)

Edit: Look at axios/axios#4659 (comment)

@SamueleCaprioli97
Copy link
Author

Hi @arthurfiorette, thank you for spending time on this issue.
Here, i've tried to update my axios dependency to 1.0.0-alpha.1 which includes your commits, but i've encountered the same behavior.

I think it's related to this portion of code in Axios:
https://github.com/axios/axios/blob/3cf6ad72033fd6eacf720a7d700f99dc96f586a3/lib/core/dispatchRequest.js#L10-L31

When a request is aborted before it is crafted, the throwIfCancellationRequested function will not pass the config as parameter to CanceledError


Anyway i found a temporary workaround for handling aborted request that makes me able to go ahead:

import axios from 'axios'
import { setupCache, buildMemoryStorage, defaultKeyGenerator } from 'axios-cache-interceptor'
const MINUTE = 1000 * 60
const memoryStorage = buildMemoryStorage()
const instance = setupCache(axios, {
  methods: ['get', 'options'],
  ttl: MINUTE,
  storage: memoryStorage
})
instance.interceptors.request.use(function (config) {
  if (!config.cache) {
    return config
  }
  let aborted = false
  if (config.cancelToken) {
    if (axios.isCancel(config.cancelToken.reason)) {
      aborted = true
    } else {
      config.cancelToken.subscribe(() => {
        abortCacheHandler(config)
      })
    }
  }
  if (config.signal) {
    if (config.signal.aborted) {
      aborted = true
    } else {
      config.signal.addEventListener('abort', () => abortCacheHandler(config), { once: true })
    }
  }
  if (aborted) {
    return { ...config, ...{ cache: false } }
  }
  return config
})
function abortCacheHandler (config) {
  const requestKey = defaultKeyGenerator(config)
  memoryStorage.get(requestKey, config).then(request => {
    if (request.state !== 'cached') {
      memoryStorage.remove(requestKey, config)
    }
  })
}

@arthurfiorette arthurfiorette added axios related Issues that are more related to axios than to axios-cache-interceptor. and removed bug Something isn't working labels Aug 28, 2022
@arthurfiorette arthurfiorette changed the title bug: Unexpected behavior while aborting a new request bug: Request doesn't execute after a abortion. Sep 9, 2022
@arthurfiorette
Copy link
Owner

arthurfiorette commented Sep 9, 2022

Hey @SamueleCaprioli97. I will close this issue as it is not related to something axios-cache-interceptor can do to fix it.

I polished the above code into a more suitable one, you can use it:

import Axios from 'axios';
import { setupCache } from './cache/create';

const axios = setupCache(Axios.create());

axios.interceptors.request.use((config) => {
  if (!config.cache) {
    return config;
  }

  async function reject() {
    const key = config.id ?? axios.generateKey(config);

    const storage = await axios.storage.get(key, config);

    // Request cancelled but response is already cached
    if (storage.state === 'cached' || storage.state === 'stale') {
      return;
    }

    await axios.storage.remove(key, config);
  }

  if (config.signal) {
    // Already cancelled request
    if (config.signal.aborted) {
      config.cache = false;
      return config;
    }

    // eslint-disable-next-line @typescript-eslint/no-misused-promises
    config.signal.addEventListener('abort', reject);
  }

  // NOTE: Cancel token is DEPRECATED but are here for backward compatibility
  // you can remove this block if you don't use cancel token.
  // https://axios-http.com/docs/cancellation
  if (config.cancelToken) {
    // Already cancelled request
    if (Axios.isCancel(config.cancelToken.reason)) {
      config.cache = false;
      return config;
    }

    void config.cancelToken.promise.then(reject);
  }

  return config;
});

@arthurfiorette
Copy link
Owner

Hey @SamueleCaprioli97, I reopened this PR because axios just released v1.0.0.
I added this fix to be in our roadmap :)

@arthurfiorette arthurfiorette reopened this Oct 5, 2022
@arthurfiorette
Copy link
Owner

Waiting for axios/axios#4922

@arthurfiorette
Copy link
Owner

Fixed! Going live with v1.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
axios related Issues that are more related to axios than to axios-cache-interceptor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants