Skip to content

Commit

Permalink
fix: infinite loading states when a restart occurs in the middle of a…
Browse files Browse the repository at this point in the history
… request. (#612)
  • Loading branch information
arthurfiorette committed Sep 3, 2023
1 parent b352f12 commit 130ef0d
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 9 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"format": "prettier --write .",
"lint": "eslint . --ext .ts",
"test": "NODE_V8_COVERAGE=coverage node -r ./dist/test/setup.js --enable-source-maps --trace-warnings --experimental-test-coverage --test dist/test/**/*.test.js",
"test:only": "tsc -p tsconfig.test.json && NODE_V8_COVERAGE=coverage node -r ./dist/test/setup.js --enable-source-maps --trace-warnings --experimental-test-coverage --test-only",
"version": "auto-changelog -p && cp CHANGELOG.md docs/src/others/changelog.md && git add CHANGELOG.md docs/src/others/changelog.md"
},
"resolutions": {
Expand Down
2 changes: 1 addition & 1 deletion src/interceptors/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CacheAxiosResponse, InternalCacheRequestConfig } from '../cache/ax

/** See {@link AxiosInterceptorManager} */
export interface AxiosInterceptor<T> {
onFulfilled?(value: T): T | Promise<T>;
onFulfilled(value: T): T | Promise<T>;

/** Returns a successful response or re-throws the error */
onRejected?(error: Record<string, unknown>): T | Promise<T>;
Expand Down
17 changes: 9 additions & 8 deletions src/interceptors/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,10 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
if (cache.state === 'loading') {
const deferred = axios.waiting[config.id];

// Just in case, the deferred doesn't exists.
/* istanbul ignore if 'really hard to test' */
// The deferred may not exists when the process is using a persistent
// storage and cancelled in the middle of a request, this would result in
// a pending loading state in the storage but no current promises to resolve
if (!deferred) {
await axios.storage.remove(config.id, config);

// Hydrates any UI temporarily, if cache is available
if (cache.data) {
await config.cache.hydrate?.(cache);
Expand Down Expand Up @@ -201,17 +200,18 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
await config.cache.hydrate?.(cache);
}

// The deferred is rejected when the request that we are waiting rejected cache.
return config;
// The deferred is rejected when the request that we are waiting rejects its cache.
// In this case, we need to redo the request all over again.
return onFulfilled(config);
}
} else {
cachedResponse = cache.data;
}

// Even though the response interceptor receives this one from here,
// it has been configured to ignore cached responses = true
config.adapter = (): Promise<CacheAxiosResponse> =>
Promise.resolve({
config.adapter = function cachedAdapter(): Promise<CacheAxiosResponse> {
return Promise.resolve({
config,
data: cachedResponse.data,
headers: cachedResponse.headers,
Expand All @@ -221,6 +221,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
id: config.id!
});
};

if (__ACI_DEV__) {
axios.debug?.({
Expand Down
55 changes: 55 additions & 0 deletions test/interceptors/response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,59 @@ describe('Response Interceptor', () => {

await assert.rejects(promise, error);
});

it('Cancelled deferred still should save cache after new response', async () => {
const axios = mockAxios();

const id = '1';
const controller = new AbortController();

const cancelled = axios.get('url', { id, signal: controller.signal });
const promise = axios.get('url', { id });

controller.abort();

// p1 should fail as it was aborted
try {
await cancelled;
assert.fail('should have thrown an error');
} catch (error: any) {
assert.equal(error.code, 'ERR_CANCELED');
}

const response = await promise;

// p2 should succeed as it was not aborted
await assert.ok(response.data);
await assert.equal(response.cached, false);

const storage = await axios.storage.get(id);

// P2 should have saved the cache
// even that his origin was from a cancelled deferred
assert.equal(storage.state, 'cached');
assert.equal(storage.data?.data, true);
});

it('Response gets cached even if there is a pending request without deferred.', async () => {
const axios = mockAxios();

const id = '1';

// Simulates previous unresolved request
await axios.storage.set(id, {
state: 'loading',
previous: 'empty'
});

const response = await axios.get('url', { id });

assert.equal(response.cached, false);
assert.ok(response.data);

const storage = await axios.storage.get(id);

assert.equal(storage.state, 'cached');
assert.equal(storage.data?.data, true);
});
});

0 comments on commit 130ef0d

Please sign in to comment.