Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Token should retry after refresh failure to avoid eventual client disconnection from the editing session #16890

Merged
merged 5 commits into from
Aug 21, 2024
Merged

Token should retry after refresh failure to avoid eventual client disconnection from the editing session #16890

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ef31431
WIP
oleq Aug 8, 2024
51f5a52
Merge branch 'master' into cc/6561-token-refresh-failure
oleq Aug 13, 2024
223443f
Tests and code refactoring.
oleq Aug 13, 2024
c1bd2cf
Docs: Updated Token and tokenUrl API docs.
oleq Aug 13, 2024
348d08f
Docs.
oleq Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions packages/ckeditor5-cloud-services/src/token/token.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import { ObservableMixin, CKEditorError } from 'ckeditor5/src/utils.js';
import type { TokenUrl } from '../cloudservicesconfig.js';

const DEFAULT_OPTIONS = { autoRefresh: true };
const DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME = 3600000;
const DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME = 3600000; // 1 hour
const TOKEN_FAILED_REFRESH_TIMEOUT_TIME = 5000; // 5 seconds

/**
* Class representing the token used for communication with CKEditor Cloud Services.
Expand All @@ -39,6 +40,7 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() {
private _options: { initValue?: string; autoRefresh: boolean };

private _tokenRefreshTimeout?: ReturnType<typeof setTimeout>;
private _tokenFailedRefreshTimeout?: ReturnType<typeof setTimeout>;

/**
* Creates `Token` instance.
Expand Down Expand Up @@ -112,14 +114,29 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() {
}

return this as InitializedToken;
} )
.catch( err => {
/**
* TODO
*
* @error token-refresh-failed
*/
console.warn( 'token-refresh-failed: TODO' );
DawidKossowski marked this conversation as resolved.
Show resolved Hide resolved

// If the refresh failed, keep trying to refresh the token. Failing to do so will eventually
// lead to the disconnection from the RTC service and the editing session (and potential data loss
// if the user keeps editing).
this._registerFailedRefreshTokenTimeout();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should engage only when using the default refresh callback. A custom integrator's callback could come with its own refresh logic and this would be redundant or even buggy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured out that it's unlikely for a custom refresh callback to come with a retry logic and integrators who fall into this category should also benefit from auto-retry logic.


return err;
} );
}

/**
* Destroys token instance. Stops refreshing.
*/
public destroy(): void {
clearTimeout( this._tokenRefreshTimeout );
this._clearRefreshTimeouts();
}

/**
Expand Down Expand Up @@ -154,13 +171,33 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() {
private _registerRefreshTokenTimeout() {
const tokenRefreshTimeoutTime = this._getTokenRefreshTimeoutTime();

clearTimeout( this._tokenRefreshTimeout );
this._clearRefreshTimeouts();

this._tokenRefreshTimeout = setTimeout( () => {
console.log( 'Refreshing token due to expiry time...' );

this.refreshToken();
}, tokenRefreshTimeoutTime );
}

/**
* TODO
*/
private _registerFailedRefreshTokenTimeout() {
this._clearRefreshTimeouts();

this._tokenFailedRefreshTimeout = setTimeout( () => {
console.log( 'Refreshing token after a failure...' );

this.refreshToken()
.then( () => {
// If refresh was successful, the logic will switch to the default timeout (if enabled)
// based on token expiry time and this timeout will no longer be needed.
clearTimeout( this._tokenFailedRefreshTimeout );
} );
}, TOKEN_FAILED_REFRESH_TIMEOUT_TIME );
}

/**
* Returns token refresh timeout time calculated from expire time in the token payload.
*
Expand All @@ -171,6 +208,8 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() {
const [ , binaryTokenPayload ] = this.value!.split( '.' );
const { exp: tokenExpireTime } = JSON.parse( atob( binaryTokenPayload ) );

console.log( 'Token expiry time', new Date( tokenExpireTime * 1000 ) );

if ( !tokenExpireTime ) {
return DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME;
}
Expand All @@ -194,6 +233,14 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() {

return token.init();
}

/**
* TODO
*/
private _clearRefreshTimeouts() {
clearTimeout( this._tokenRefreshTimeout );
clearTimeout( this._tokenFailedRefreshTimeout );
}
}

/**
Expand Down