From f419798e8e32e98fdbb093ca79421ca8458e838e Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Fri, 24 Jul 2026 10:54:31 -0700 Subject: [PATCH] fix: Exponentially back off initialization-error retries (#2579) (#2605) - Closes #2579 --- .../issues/issues/initialization.ts | 65 +++++++++++--- .../issues/issues/initialization.test.ts | 90 +++++++++++++++++-- 2 files changed, 140 insertions(+), 15 deletions(-) diff --git a/src/card-controller/issues/issues/initialization.ts b/src/card-controller/issues/issues/initialization.ts index c81905f7..090c3ab4 100644 --- a/src/card-controller/issues/issues/initialization.ts +++ b/src/card-controller/issues/issues/initialization.ts @@ -16,27 +16,72 @@ export class InitializationIssue extends AbstractErrorIssue { private _api: CardIssueManagerAPI; + // True while a re-attempt dispatched by retry() is in flight, awaiting its + // verdict. retry() has to clear _error up front, because the init error is a + // full-card issue and uniquely initialization refuses to run while one is + // shown (for good reason), so the error cannot double as the "still + // unresolved" signal across the re-attempt. This flag carries that signal + // instead: it keeps needsRetry() true across the gap so the backoff keeps + // escalating rather than restarting from the base delay each cycle. Cleared + // when the re-attempt settles -- a failure (trigger) or a success + // (detectDynamic). + // See: https://github.com/dermotduffy/advanced-camera-card/issues/2579 + private _retrying = false; + constructor(api: CardIssueManagerAPI) { super(); this._api = api; } - public detectDynamic(): void { - if ( - this._error !== null && - this._api.getInitializationManager().isInitializedMandatory() - ) { - this._error = null; - } + // A fresh failure: the re-attempt has settled, so leave the in-flight state + // before recording the new error. + public trigger(context: { error: unknown }): void { + this._retrying = false; + super.trigger(context); } + // Clearing the error also abandons any in-flight re-attempt. + public reset(): void { + this._retrying = false; + super.reset(); + } + + public detectDynamic(): void { + if (!this._api.getInitializationManager().isInitializedMandatory()) { + return; + } + // The success settle: mandatory init completed, so there is no error to show + // and no re-attempt outstanding. Clearing both lets needsRetry() go false and + // the backoff reset. + this._error = null; + this._retrying = false; + } + + // Unresolved while an error is showing, or while a dispatched re-attempt is + // still awaiting its verdict. Kept true across that gap so the backoff is not + // reset mid-sequence. public needsRetry(): boolean { - return this._error !== null; + return this._error !== null || this._retrying; + } + + // Dispatch a retry only when there is an error to act on and no re-attempt is + // already running; otherwise wait for the current one to settle. + public canRetryNow(): boolean { + return this._error !== null && !this._retrying; } public retry(): boolean { - // Clear the error so the full-card issue is removed and shouldUpdate() - // no longer short-circuits before initializeMandatory(). + if (this._retrying) { + // Already awaiting a verdict. Only a forced (user) retry reaches here; + // scheduled ones are gated by canRetryNow(). Do not tear it down and + // restart it. + return false; + } + + // Enter the in-flight state and clear the error. Removing the error tears + // down the full-card issue, which is what unblocks initialization from + // re-attempting (it refuses to start while a full-card issue is shown). + this._retrying = true; this._error = null; // Reset init state so initializeMandatory() re-attempts on the next diff --git a/tests/card-controller/issues/issues/initialization.test.ts b/tests/card-controller/issues/issues/initialization.test.ts index b603786c..59c406dc 100644 --- a/tests/card-controller/issues/issues/initialization.test.ts +++ b/tests/card-controller/issues/issues/initialization.test.ts @@ -81,7 +81,7 @@ describe('InitializationIssue', () => { }); describe('detectDynamic', () => { - it('should clear the issue when initialization is now mandatory', () => { + it('should clear the issue when mandatory init completes', () => { const issue = new InitializationIssue(createAPI(true)); issue.trigger({ error: new Error('init failed') }); expect(issue.hasIssue()).toBe(true); @@ -92,7 +92,7 @@ describe('InitializationIssue', () => { expect(issue.getIssue()).toBeNull(); }); - it('should keep the issue when initialization is still not mandatory', () => { + it('should keep the issue when mandatory init is not complete', () => { const issue = new InitializationIssue(createAPI(false)); issue.trigger({ error: new Error('init failed') }); @@ -101,14 +101,26 @@ describe('InitializationIssue', () => { expect(issue.hasIssue()).toBe(true); }); - it('should do nothing when not failed', () => { + it('should do nothing when mandatory init is not complete', () => { const api = createAPI(false); const issue = new InitializationIssue(api); issue.detectDynamic(); expect(issue.hasIssue()).toBe(false); - expect(api.getInitializationManager().isInitializedMandatory).not.toBeCalled(); + }); + + it('should end an in-flight retry when mandatory init completes', () => { + const api = createAPI(true); + const issue = new InitializationIssue(api); + issue.trigger({ error: new Error('init failed') }); + issue.retry(); + expect(issue.needsRetry()).toBe(true); + + issue.detectDynamic(); + + expect(issue.needsRetry()).toBe(false); + expect(issue.canRetryNow()).toBe(false); }); }); @@ -123,6 +135,49 @@ describe('InitializationIssue', () => { const issue = new InitializationIssue(createAPI()); expect(issue.needsRetry()).toBe(false); }); + + it('should stay true across an in-flight retry so the backoff is not reset', () => { + const issue = new InitializationIssue(createAPI()); + issue.trigger({ error: new Error('init failed') }); + + issue.retry(); + + expect(issue.hasIssue()).toBe(false); + expect(issue.needsRetry()).toBe(true); + }); + + it('should become true again after a fresh failure re-triggers', () => { + const issue = new InitializationIssue(createAPI()); + issue.trigger({ error: new Error('init failed') }); + issue.retry(); + + issue.trigger({ error: new Error('init failed again') }); + + expect(issue.needsRetry()).toBe(true); + expect(issue.canRetryNow()).toBe(true); + }); + }); + + describe('canRetryNow', () => { + it('should return false when not failed', () => { + const issue = new InitializationIssue(createAPI()); + expect(issue.canRetryNow()).toBe(false); + }); + + it('should return true when failed and not in flight', () => { + const issue = new InitializationIssue(createAPI()); + issue.trigger({ error: new Error('init failed') }); + expect(issue.canRetryNow()).toBe(true); + }); + + it('should return false while a retry is in flight', () => { + const issue = new InitializationIssue(createAPI()); + issue.trigger({ error: new Error('init failed') }); + + issue.retry(); + + expect(issue.canRetryNow()).toBe(false); + }); }); describe('retry', () => { @@ -135,10 +190,24 @@ describe('InitializationIssue', () => { expect(result).toBe(false); expect(issue.hasIssue()).toBe(false); - expect(issue.needsRetry()).toBe(false); expect(api.getInitializationManager().uninitializeMandatory).toBeCalled(); expect(api.getCameraManager().destroy).toBeCalled(); }); + + it('should be a no-op while a retry is already in flight', () => { + const api = createAPI(); + const issue = new InitializationIssue(api); + issue.trigger({ error: new Error('init failed') }); + issue.retry(); + vi.mocked(api.getInitializationManager().uninitializeMandatory).mockClear(); + vi.mocked(api.getCameraManager().destroy).mockClear(); + + const result = issue.retry(); + + expect(result).toBe(false); + expect(api.getInitializationManager().uninitializeMandatory).not.toBeCalled(); + expect(api.getCameraManager().destroy).not.toBeCalled(); + }); }); it('should clear the issue after reset', () => { @@ -151,4 +220,15 @@ describe('InitializationIssue', () => { expect(issue.hasIssue()).toBe(false); expect(issue.getIssue()).toBeNull(); }); + + it('should end an in-flight retry after reset', () => { + const issue = new InitializationIssue(createAPI()); + issue.trigger({ error: new Error('oops') }); + issue.retry(); + expect(issue.needsRetry()).toBe(true); + + issue.reset(); + + expect(issue.needsRetry()).toBe(false); + }); });