Skip to content

Commit

Permalink
Merge pull request #2595 from reduxjs/bugfix/settimeout-max-value
Browse files Browse the repository at this point in the history
  • Loading branch information
markerikson committed Aug 14, 2022
2 parents f768913 + e67b93d commit dc673a3
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
Expand Up @@ -28,6 +28,13 @@ declare module '../../endpointDefinitions' {
}
}

// Per https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value , browsers store
// `setTimeout()` timer values in a 32-bit int. If we pass a value in that's larger than that,
// it wraps and ends up executing immediately.
// Our `keepUnusedDataFor` values are in seconds, so adjust the numbers here accordingly.
export const THIRTY_TWO_BIT_MAX_INT = 2_147_483_647
export const THIRTY_TWO_BIT_MAX_TIMER_SECONDS = 2_147_483_647 / 1_000 - 1

export const build: SubMiddlewareBuilder = ({ reducerPath, api, context }) => {
const { removeQueryResult, unsubscribeQueryResult } = api.internalActions

Expand Down Expand Up @@ -87,6 +94,14 @@ export const build: SubMiddlewareBuilder = ({ reducerPath, api, context }) => {
] as QueryDefinition<any, any, any, any>
const keepUnusedDataFor =
endpointDefinition?.keepUnusedDataFor ?? config.keepUnusedDataFor
// Prevent `setTimeout` timers from overflowing a 32-bit internal int, by
// clamping the max value to be at most 1000ms less than the 32-bit max.
// Look, a 24.8-day keepalive ought to be enough for anybody, right? :)
// Also avoid negative values too.
const finalKeepUnusedDataFor = Math.max(
0,
Math.min(keepUnusedDataFor, THIRTY_TWO_BIT_MAX_TIMER_SECONDS)
)

const currentTimeout = currentRemovalTimeouts[queryCacheKey]
if (currentTimeout) {
Expand All @@ -99,7 +114,7 @@ export const build: SubMiddlewareBuilder = ({ reducerPath, api, context }) => {
api.dispatch(removeQueryResult({ queryCacheKey }))
}
delete currentRemovalTimeouts![queryCacheKey]
}, keepUnusedDataFor * 1000)
}, finalKeepUnusedDataFor * 1000)
}
}
}
33 changes: 33 additions & 0 deletions packages/toolkit/src/query/tests/cacheCollection.test.ts
Expand Up @@ -2,6 +2,10 @@ import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query'
import { configureStore } from '@reduxjs/toolkit'
import { waitMs } from './helpers'
import type { Middleware, Reducer } from 'redux'
import {
THIRTY_TWO_BIT_MAX_INT,
THIRTY_TWO_BIT_MAX_TIMER_SECONDS,
} from '../core/buildMiddleware/cacheCollection'

beforeAll(() => {
jest.useFakeTimers('legacy')
Expand Down Expand Up @@ -52,6 +56,35 @@ test(`query: await cleanup, keepUnusedDataFor set`, async () => {
expect(onCleanup).toHaveBeenCalled()
})

test(`query: handles large keepUnuseDataFor values over 32-bit ms`, async () => {
const { store, api } = storeForApi(
createApi({
baseQuery: fetchBaseQuery({ baseUrl: 'https://example.com' }),
endpoints: (build) => ({
query: build.query<unknown, string>({
query: () => '/success',
}),
}),
keepUnusedDataFor: THIRTY_TWO_BIT_MAX_TIMER_SECONDS - 10,
})
)

store.dispatch(api.endpoints.query.initiate('arg')).unsubscribe()

// Shouldn't have been called right away
jest.advanceTimersByTime(1000), await waitMs()
expect(onCleanup).not.toHaveBeenCalled()

// Shouldn't have been called any time in the next few minutes
jest.advanceTimersByTime(1_000_000), await waitMs()
expect(onCleanup).not.toHaveBeenCalled()

// _Should_ be called _wayyyy_ in the future (like 24.8 days from now)
jest.advanceTimersByTime(THIRTY_TWO_BIT_MAX_TIMER_SECONDS * 1000),
await waitMs()
expect(onCleanup).toHaveBeenCalled()
})

describe(`query: await cleanup, keepUnusedDataFor set`, () => {
const { store, api } = storeForApi(
createApi({
Expand Down

0 comments on commit dc673a3

Please sign in to comment.