fix: Improve token schedule to avoid session close #202
Conversation
| tag: 'AUTH', | ||
| }); | ||
|
|
||
| ConfigStore.set<EncryptedTokenKey>(`${tokenName}Encrypted`, false); |
There was a problem hiding this comment.
Why are we setting the token now when before where throwing an expection? What changed?
There was a problem hiding this comment.
Previously, the strategy was to throw an exception when encryption was unavailable. Now, the token is marked as unencrypted and returned, allowing the request to execute without interruption. The application currently stores tokens in plain text when safeStorage is unavailable, so throwing an exception is unnecessary since the token remains usable. If the token becomes corrupted for any reason, the request utilizing it would trigger an onUserUnauthorized exception, forcing the user to log in again and replacing the corrupted token.
| } | ||
|
|
||
| export async function createTokenSchedule(refreshedTokens?: Array<string | undefined>): Promise<void> { | ||
| export async function createTokenSchedule(refreshedTokens?: Array<string | undefined>) { |
There was a problem hiding this comment.
Since this method now has a retry mechanism, you could add it onto the name of the funtion to better reflect the behaviour
There was a problem hiding this comment.
Or better yet: split into two and have one call the other, this way you have better separation of concerns
There was a problem hiding this comment.
I think renaming it would be a better approach. The current function is only about 10 lines long, so further breaking it down wouldn't provide significant value.
| import { TokenScheduler } from '../../token-scheduler/TokenScheduler'; | ||
| import { Job } from 'node-schedule'; | ||
|
|
||
| describe('createTokenScheduleWithRetry', () => { |
There was a problem hiding this comment.
in this case, you are naming this test file with a file that it does not exists, but rather is a method from another file. You have 2 options here, either move that method to a separate file, or in the refreshToken test have this whole section inside, so you would have it like this
describe('refreshToken', () => {
[...]
describe('createTokenScheduleWithRetry', () => {
[...]
});
});
``There was a problem hiding this comment.
The functions have been placed in separate files to make it more organized.
| @@ -0,0 +1,42 @@ | |||
| import { Either, left, right } from '../../../../context/shared/domain/Either'; | |||
There was a problem hiding this comment.
The functions have been placed in separate files to make it more organized.
|
What is Changed / Added
There were some inconsistencies in the validations responsible for refreshing the token, which caused unexpected shutdowns in the application session.
Why