🟢 Code Quality: Good

The error detection functions are pure, focused, and readable.

⚠️ However, string-inclusion matching on error messages is inherently brittle. If Chromium ever changes the exact error string, the heal logic silently stops working.

🟡 Issue - Fragile string matching: The heal logic relies on exact error message substrings. Consider adding a fallback or a more robust pattern.

// Current (brittle):
return error instanceof Error && error.message.includes('Internal error opening backing store');

// Suggested: slightly more resilient, though still message-based
const BACKING_STORE_PATTERNS = ['internal error opening backing store', 'backing store'];
// ...
return BACKING_STORE_PATTERNS.some(p => msg.includes(p));

Why it matters: If a browser vendor changes the error wording slightly (which happens), the 884K/month errors will continue to crash the app despite having heal logic that should catch them.

This can end up include other class of errors like QuotaExceededError: Encountered full disk while opening backing store for indexedDB.open..

I agree that Chrome could change the errors some day but we can't really protect ourselves against this, it could change to something completely different from backing store and the suggestion would still fail.

Using msg.includes('connection is closing') means we will include other connection closing errors too, e.g.:

• InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing. (initially handled in fix the database connection is closing issue. #748)
• UnknownError: Connection is closing because of: IO error:
• UnknownError: Connection is closing because of: Failed to remove blob file.
• UnknownError: Connection is closing.
• UnknownError: Connection is closing because of: Force close delete origin
• UnknownError: Connection is closing because of: Corruption: block checksum mismatch

It's intended, right?

yes, it's intended - for all unexpected errors from this family

🟡 Potential edge case: InvalidStateError type checking relaxed

The old code checked:

if (error instanceof DOMException && error.name === 'InvalidStateError') {

The new code uses:

function isInvalidStateError(error: unknown): boolean {
    return error instanceof Error && error.name === 'InvalidStateError';
}

This is more permissive (DOMException is not strictly instanceof Error in all envs, but Error is broader). In practice this is unlikely to cause issues since DOMException typically inherits from Error. But if an env throws an InvalidStateError that is not an Error, this check would miss it.

Suggested fix - tighten the check:

function isInvalidStateError(error: unknown): boolean {
    return (error instanceof Error || error instanceof DOMException) && (error as Error).name === 'InvalidStateError';
}

🟡 Issue: getBudgetedHealErrorLabel is fragile to future categories

Similar to here, this assumes that if it's not a backing store error, it MUST be a connection lost error. If a third budgeted heal error category is ever added to isBudgetedHealError, this will silently mislabel it as 'connection lost'.

Suggested fix:

function getBudgetedHealErrorLabel(error: unknown): 'backing store' | 'connection lost' | null {
    if (isBackingStoreError(error)) return 'backing store';
    if (isConnectionLostError(error)) return 'connection lost';
    return null;
}   

And update the caller:

const label = getBudgetedHealErrorLabel(error);
if (!label) {
    throw new Error('Unexpected: isBudgetedHealError matched but no label found');
}

Agree but we shouldn't throw an Error just because the label wasn't found, let's just log something and/or skip the logic if necessary

Is there a reason not to keep the counter local and per-transaction? It seems we'll have one variable for the whole app?