AlbinoDrought / cachios

Simple axios cache wrapper using node-cache
MIT License
88 stars 10 forks source link

Workaround #55 by preventing unresolved cancellable requests from being shared #57

Closed AlbinoDrought closed 3 years ago

AlbinoDrought commented 3 years ago

Fixes #55

Cause

A mechanism was added in 0368696eb8f7a9181e37adef6d19e05bee7a5aab to prevent sending duplicate identical requests.

If there was a server running this:

const express = require('express');

const app = express();
app.get('/', (req, res) => {
  // send "hello" after 1 second
  console.log('hit');
  setTimeout(() => res.send('hello'), 1000);
});

app.listen(8080);

And a user did this:

const cachios = require('cachios');

const requests = [
  cachios.get('http://localhost:8080'),
  cachios.get('http://localhost:8080'),
  cachios.get('http://localhost:8080'),
  cachios.get('http://localhost:8080'),
  cachios.get('http://localhost:8080'),
];

Promise.all(requests).then(console.log);

Only one request would actually be made to the server. Internally, the promise from the first axios.get('http://localhost:8080') would be shared across all of these calls.

This runs into issues once cancelTokens are introduced (#55). Although Cachios stops sharing these promises once they're rejected, code that:

  1. Cancels a token for an unresolved request
  2. Doesn't yield to the event loop
  3. Recreates an identical request

Runs before the Cachios reject handler has been fired:

const axios = require('axios');

const get = (url) => {
  console.log('cachios shares promises here');
  return axios.get(url);
};

let cancelToken;

const getData = (i) =>{
  if (cancelToken) {
    cancelToken.cancel();
  }
  cancelToken = axios.CancelToken.source();
  return get('http://localhost:8080', {
    cancelToken: cancelToken.token
  }).then(() => {
    console.log('promise resolved', i);
  }).catch((thrown) => {
    console.log('promise rejected', i);
  });
}
getData(1);
getData(2);

Output:

cachios shares promises here cachios shares promises here promise rejected 1 promise resolved 2

Workaround

We can avoid this issue by disabling the sharing of unresolved cancellable requests. Requests that provide a cancelToken property in their request config will never share their promise or use a shared promise.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling c609f2b1f7ff67b8a9f55c51209e9a83324d1127 on bugfix/55/never-cache-cancelled-requests into 95e14b5e6f1a0aa95b45ae2ef8e471f639736379 on master.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling c609f2b1f7ff67b8a9f55c51209e9a83324d1127 on bugfix/55/never-cache-cancelled-requests into 95e14b5e6f1a0aa95b45ae2ef8e471f639736379 on master.