From 33dab8fa79b4336e8628a2a8f7022436388d20d0 Mon Sep 17 00:00:00 2001 From: f1ames Date: Thu, 14 Nov 2024 13:59:03 +0100 Subject: [PATCH] Fix aborting refresh token logic in specific cases. --- .../src/token/token.ts | 15 +++- .../tests/token/token.js | 84 ++++++++++++++++++- 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-cloud-services/src/token/token.ts b/packages/ckeditor5-cloud-services/src/token/token.ts index b560a6b1078..3fead48b56b 100644 --- a/packages/ckeditor5-cloud-services/src/token/token.ts +++ b/packages/ckeditor5-cloud-services/src/token/token.ts @@ -48,6 +48,11 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { */ private _tokenRefreshTimeout?: ReturnType; + /** + * Flag indicating whether the token has been destroyed. + */ + private _isDestroyed = false; + /** * Creates `Token` instance. * Method `init` should be called after using the constructor or use `create` method instead. @@ -163,6 +168,8 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * Destroys token instance. Stops refreshing. */ public destroy(): void { + this._isDestroyed = true; + clearTimeout( this._tokenRefreshTimeout ); } @@ -196,10 +203,14 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * Registers a refresh token timeout for the time taken from token. */ private _registerRefreshTokenTimeout( timeoutTime?: number ) { - const tokenRefreshTimeoutTime = timeoutTime || this._getTokenRefreshTimeoutTime(); - clearTimeout( this._tokenRefreshTimeout ); + if ( this._isDestroyed ) { + return; + } + + const tokenRefreshTimeoutTime = timeoutTime || this._getTokenRefreshTimeoutTime(); + this._tokenRefreshTimeout = setTimeout( () => { this.refreshToken(); }, tokenRefreshTimeoutTime ); diff --git a/packages/ckeditor5-cloud-services/tests/token/token.js b/packages/ckeditor5-cloud-services/tests/token/token.js index a89ce43d769..7f709a67a03 100644 --- a/packages/ckeditor5-cloud-services/tests/token/token.js +++ b/packages/ckeditor5-cloud-services/tests/token/token.js @@ -216,8 +216,17 @@ describe( 'Token', () => { } ); describe( 'destroy', () => { + let clock; + + beforeEach( () => { + clock = sinon.useFakeTimers(); + } ); + + afterEach( () => { + clock.restore(); + } ); + it( 'should stop refreshing the token', async () => { - const clock = sinon.useFakeTimers(); const tokenInitValue = getTestTokenValue(); const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue } ); @@ -239,8 +248,79 @@ describe( 'Token', () => { await clock.tickAsync( 3600000 ); expect( requests.length ).to.equal( 2 ); + } ); - clock.restore(); + // See https://github.com/ckeditor/ckeditor5/issues/17462. + it( 'should stop refreshing the token when editor destroyed before token request rejects', async () => { + const consoleWarnStub = testUtils.sinon.stub( console, 'warn' ); + + const token = new Token( 'http://token-endpoint', { autoRefresh: true } ); + const registerRefreshTokenTimeoutSpy = testUtils.sinon.spy( token, '_registerRefreshTokenTimeout' ); + + const initPromise = token.init(); + + // Editor is destroyed before the token request is resolved. + token.destroy(); + + // Simulate token fetching error. + requests[ 0 ].error(); + + try { + await initPromise; + throw new Error( 'Promise should be rejected' ); + } catch ( error ) { + expect( error ).to.match( /Network Error/ ); + } + + sinon.assert.calledOnce( registerRefreshTokenTimeoutSpy ); + sinon.assert.calledOnce( consoleWarnStub ); + expect( requests.length ).to.equal( 1 ); + + await clock.tickAsync( 10 * 1000 ); + + sinon.assert.calledOnce( registerRefreshTokenTimeoutSpy ); + sinon.assert.calledOnce( consoleWarnStub ); + expect( requests.length ).to.equal( 1 ); + + await clock.tickAsync( 3600000 ); + await clock.tickAsync( 3600000 ); + + sinon.assert.calledOnce( registerRefreshTokenTimeoutSpy ); + sinon.assert.calledOnce( consoleWarnStub ); + expect( requests.length ).to.equal( 1 ); + } ); + + // Related to https://github.com/ckeditor/ckeditor5/issues/17462. + it( 'should stop refreshing the token when editor destroyed before token request resolves', async () => { + const token = new Token( 'http://token-endpoint', { autoRefresh: true } ); + const registerRefreshTokenTimeoutSpy = testUtils.sinon.spy( token, '_registerRefreshTokenTimeout' ); + + const initPromise = token.init(); + + // Editor is destroyed before the token request is resolved. + token.destroy(); + + // Simulate token response. + requests[ 0 ].respond( 200, '', getTestTokenValue( 1500 ) ); + + try { + await initPromise; + } catch ( error ) { + throw new Error( 'Promise should not be rejected' ); + } + + sinon.assert.calledOnce( registerRefreshTokenTimeoutSpy ); + expect( requests.length ).to.equal( 1 ); + + await clock.tickAsync( 3600000 ); + + sinon.assert.calledOnce( registerRefreshTokenTimeoutSpy ); + expect( requests.length ).to.equal( 1 ); + + await clock.tickAsync( 3600000 ); + + sinon.assert.calledOnce( registerRefreshTokenTimeoutSpy ); + expect( requests.length ).to.equal( 1 ); } ); } );