From e2b73e1c3a15bd2730d84e2a8516d6d2c3d00147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9?= <98621042+joseiedo@users.noreply.github.com> Date: Sun, 30 Jul 2023 03:39:36 -0300 Subject: [PATCH] fix: avoid `RangeError: Maximum call stack size exceeded` on streams and other objects (#616) * refactor: change id generation location * fix: lint * refactor: ensures id will be created only if cache is not false * chore(deps-dev): bump vitepress from 1.0.0-beta.5 to 1.0.0-beta.6 (#618) * chore(deps-dev): bump @types/node from 18.16.19 to 18.17.0 (#617) * chore(deps-dev): bump tslib from 2.6.0 to 2.6.1 (#619) * chore(deps-dev): bump @types/node from 18.17.0 to 18.17.1 (#620) * chore: dependabot * ci: dependabot * feat: handle non axios errors rejections (#609) * fix: correct config re throw * chore(deps-dev): bump eslint-config-prettier from 8.8.0 to 8.9.0 (#624) * chore(deps-dev): bump jest from 29.6.1 to 29.6.2 (#622) * feat: turn most types into interfaces (#615) * Turn most types into interfaces * Turn 'CacheAxiosResponse' into a interface * Update docs * Change docs to be more didactic * chore(deps-dev): bump jest-environment-jsdom from 29.6.1 to 29.6.2 (#623) * chore: removed unused eslint comment * feat: handle errors on object-code * feat: bring back ids * chore: lint --------- Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Arthur Fiorette Co-authored-by: Denis Rossati --- package.json | 2 +- pnpm-lock.yaml | 12 ++++++--- src/interceptors/request.ts | 33 +++++++++++------------ src/interceptors/response.ts | 46 +++++++++++++++++---------------- test/util/key-generator.test.ts | 16 ++++++++++++ 5 files changed, 66 insertions(+), 43 deletions(-) diff --git a/package.json b/package.json index 204cdf1..cd92bbc 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "dependencies": { "cache-parser": "^1.2.4", "fast-defer": "^1.1.7", - "object-code": "^1.2.4" + "object-code": "^1.3.0" }, "devDependencies": { "@arthurfiorette/prettier-config": "*", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cbddd29..237723e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,5 +1,9 @@ lockfileVersion: '6.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + overrides: colors: 1.4.0 @@ -11,8 +15,8 @@ dependencies: specifier: ^1.1.7 version: 1.1.7 object-code: - specifier: ^1.2.4 - version: 1.2.4 + specifier: ^1.3.0 + version: 1.3.0 devDependencies: '@arthurfiorette/prettier-config': @@ -4359,8 +4363,8 @@ packages: resolution: {integrity: sha512-6xpotnECFy/og7tKSBVmUNft7J3jyXAka4XvG6AUhFWRz+Q/Ljus7znJAA3bxColfQLdS+XsjoodtJfCgeTEFQ==} dev: true - /object-code@1.2.4: - resolution: {integrity: sha512-uGq4ETUuWe+GA586NXEriiaozNuff+YNFXlpD8cVrM1GoiuTZpCABP+bZCWDrvQDoCiSTyiWAFHD/HF/iwhb2w==} + /object-code@1.3.0: + resolution: {integrity: sha512-PLplgvzuFhSPBuTX/mtaXEnU3c6g7qKflVVQbV9VWEnV/34iKeAX1jeDNCKq1OgGlsnkA/NjldCzTbHxa7Wj4A==} dev: false /once@1.4.0: diff --git a/src/interceptors/request.ts b/src/interceptors/request.ts index ae5bc53..c234fb6 100644 --- a/src/interceptors/request.ts +++ b/src/interceptors/request.ts @@ -16,7 +16,7 @@ import { export function defaultRequestInterceptor(axios: AxiosCacheInstance) { const onFulfilled: RequestInterceptor['onFulfilled'] = async (config) => { - const id = (config.id = axios.generateKey(config)); + config.id = axios.generateKey(config); if (config.cache === false) { if (__ACI_DEV__) { @@ -53,7 +53,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { } // Assumes that the storage handled staled responses - let cache = await axios.storage.get(id, config); + let cache = await axios.storage.get(config.id, config); const overrideCache = config.cache.override; // Not cached, continue the request, and mark it as fetching @@ -67,8 +67,8 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { * first await statement, so the second (asynchronous call) request may have already * started executing. */ - if (axios.waiting[id] && !overrideCache) { - cache = (await axios.storage.get(id, config)) as + if (axios.waiting[config.id] && !overrideCache) { + cache = (await axios.storage.get(config.id, config)) as | CachedStorageValue | LoadingStorageValue; @@ -82,7 +82,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { if (cache.state !== 'empty') { if (__ACI_DEV__) { axios.debug?.({ - id, + id: config.id, msg: 'Waiting list had an deferred for this key, waiting for it to finish' }); } @@ -92,17 +92,17 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { } // Create a deferred to resolve other requests for the same key when it's completed - axios.waiting[id] = deferred(); + axios.waiting[config.id] = deferred(); /** * Adds a default reject handler to catch when the request gets aborted without * others waiting for it. */ // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - axios.waiting[id]!.catch(() => undefined); + axios.waiting[config.id]!.catch(() => undefined); await axios.storage.set( - id, + config.id, { state: 'loading', previous: overrideCache @@ -132,7 +132,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: config.id, msg: 'Updated stale request' }); } @@ -142,7 +142,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: config.id, msg: 'Sending request, waiting for response', data: { overrideCache, @@ -162,12 +162,12 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { let cachedResponse: CachedResponse; if (cache.state === 'loading') { - const deferred = axios.waiting[id]; + const deferred = axios.waiting[config.id]; // Just in case, the deferred doesn't exists. /* istanbul ignore if 'really hard to test' */ if (!deferred) { - await axios.storage.remove(id, config); + await axios.storage.remove(config.id, config); // Hydrates any UI temporarily, if cache is available if (cache.data) { @@ -179,7 +179,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: config.id, msg: 'Detected concurrent request, waiting for it to finish' }); } @@ -189,7 +189,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { } catch (err) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: config.id, msg: 'Deferred rejected, requesting again', data: err }); @@ -218,12 +218,13 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) { status: cachedResponse.status, statusText: cachedResponse.statusText, cached: true, - id + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + id: config.id! }); if (__ACI_DEV__) { axios.debug?.({ - id, + id: config.id, msg: 'Returning cached response' }); } diff --git a/src/interceptors/response.ts b/src/interceptors/response.ts index ce8e410..0c7479b 100644 --- a/src/interceptors/response.ts +++ b/src/interceptors/response.ts @@ -33,7 +33,7 @@ export function defaultResponseInterceptor( const onFulfilled: ResponseInterceptor['onFulfilled'] = async (response) => { // When response.config is not present, the response is indeed a error. - if (!response.config) { + if (!response?.config) { if (__ACI_DEV__) { axios.debug?.({ msg: 'Response interceptor received an unknown response.', @@ -45,14 +45,19 @@ export function defaultResponseInterceptor( throw response; } - const id = (response.id = response.config.id ??= axios.generateKey(response.config)); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + response.id = response.config.id!; response.cached ??= false; + const config = response.config; + // Request interceptor merges defaults with per request configuration + const cacheConfig = config.cache as CacheProperties; + // Response is already cached if (response.cached) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: 'Returned cached response' }); } @@ -60,22 +65,19 @@ export function defaultResponseInterceptor( return response; } - const config = response.config; - // Request interceptor merges defaults with per request configuration - const cacheConfig = config.cache as CacheProperties; - // Skip cache: either false or weird behavior // config.cache should always exists, at least from global config merge. if (!cacheConfig) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: 'Response with config.cache falsy', data: response }); } - return { ...response, cached: false }; + response.cached = false; + return response; } // Update other entries before updating himself @@ -86,7 +88,7 @@ export function defaultResponseInterceptor( if (!isMethodIn(config.method, cacheConfig.methods)) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: `Ignored because method (${config.method}) is not in cache.methods (${cacheConfig.methods})`, data: { config, cacheConfig } }); @@ -95,7 +97,7 @@ export function defaultResponseInterceptor( return response; } - const cache = await axios.storage.get(id, config); + const cache = await axios.storage.get(response.id, config); if ( // If the request interceptor had a problem or it wasn't cached @@ -103,7 +105,7 @@ export function defaultResponseInterceptor( ) { if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: "Response not cached and storage isn't loading", data: { cache, response } }); @@ -118,11 +120,11 @@ export function defaultResponseInterceptor( !cache.data && !(await testCachePredicate(response, cacheConfig.cachePredicate)) ) { - await rejectResponse(id, config); + await rejectResponse(response.id, config); if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: 'Cache predicate rejected this response' }); } @@ -156,11 +158,11 @@ export function defaultResponseInterceptor( // Cache should not be used if (expirationTime === 'dont cache') { - await rejectResponse(id, config); + await rejectResponse(response.id, config); if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: `Cache header interpreted as 'dont cache'`, data: { cache, response, expirationTime } }); @@ -191,7 +193,7 @@ export function defaultResponseInterceptor( if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: 'Useful response configuration found', data: { cacheConfig, cacheResponse: data } }); @@ -206,26 +208,26 @@ export function defaultResponseInterceptor( }; // Resolve all other requests waiting for this response - const waiting = axios.waiting[id]; + const waiting = axios.waiting[response.id]; if (waiting) { waiting.resolve(newCache.data); - delete axios.waiting[id]; + delete axios.waiting[response.id]; if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: 'Found waiting deferred(s) and resolved them' }); } } // Define this key as cache on the storage - await axios.storage.set(id, newCache, config); + await axios.storage.set(response.id, newCache, config); if (__ACI_DEV__) { axios.debug?.({ - id, + id: response.id, msg: 'Response cached', data: { cache: newCache, response } }); diff --git a/test/util/key-generator.test.ts b/test/util/key-generator.test.ts index e4a5462..c4d8d9b 100644 --- a/test/util/key-generator.test.ts +++ b/test/util/key-generator.test.ts @@ -219,4 +219,20 @@ describe('tests key generation', () => { expect(keyGenerator({ data })).not.toBe(data); expect(typeof keyGenerator({ data })).toBe('string'); }); + + it('expects key generator handles recursive objects', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const recursive: any = {}; + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access + recursive.data = recursive; + + const keyGenerator = buildKeyGenerator(({ data }) => data); + + // We should not throw errors here, as some recursive objects may be handled by axios/other interceptors + // This way, if any, error happens, it will be thrown by other packages, not this one + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + expect(() => keyGenerator(recursive)).not.toThrow(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + expect(() => defaultKeyGenerator(recursive)).not.toThrow(); + }); });