-
-
Notifications
You must be signed in to change notification settings - Fork 153
Description
When explicitly passing undefined for statementTimeout or idleInTransactionSessionTimeout, the timeouts are effectively disabled instead of falling back to the default values.
await createPool(databaseUrl, {
statementTimeout: undefined,
idleInTransactionSessionTimeout: undefined,
});
Expected behavior
Passing undefined should behave the same as omitting the option entirely, using the default timeout of 60s.
Actual behavior
The timeouts are disabled, equivalent to passing 'DISABLE_TIMEOUT'.
I believe this is coming from the following code in createClientConfiguration:
slonik/packages/slonik/src/factories/createClientConfiguration.ts
Lines 15 to 35 in a9a02c0
| const configuration = { | |
| captureStackTrace: false, | |
| connectionRetryLimit: 3, | |
| connectionTimeout: 5_000, | |
| connectionUri, | |
| dangerouslyAllowForeignConnections: false, | |
| gracefulTerminationTimeout: 5_000, | |
| idleInTransactionSessionTimeout: 60_000, | |
| idleTimeout: 5_000, | |
| interceptors: [], | |
| maximumPoolSize: 10, | |
| minimumPoolSize: 0, | |
| queryRetryLimit: 5, | |
| resetConnection: ({ query }) => { | |
| return query(`DISCARD ALL`); | |
| }, | |
| statementTimeout: 60_000, | |
| transactionRetryLimit: 5, | |
| typeParsers, | |
| ...clientUserConfigurationInput, | |
| }; |
The user config is being spread, overwriting the defaults. This would be okay, except it makes it difficult to conditionally fall back to the default values, and it's unclear whether or not it winds up behaving the same as 'DISABLE_TIMEOUT' later since it results in undefined being passed to the poolConfiguration in createPgDriverFactory (instead of being left out).
Perhaps it would be possible to use another approach to apply the user input to the defaults?
Not including a repro or test here since the code is pretty straightforward and I've referenced it here.