fix(search): bulkPut hangs instead of rejecting when an IndexedDB write fails - #637
Open
ppcvote wants to merge 2 commits into
Open
fix(search): bulkPut hangs instead of rejecting when an IndexedDB write fails#637ppcvote wants to merge 2 commits into
ppcvote wants to merge 2 commits into
Conversation
TableWrapper.bulkPut ran its work inside an async Promise executor with no reject, so a rejected chunk write settled the executor's own promise rather than the returned one. The promise never settled at all. SearchService's cold-cache path waits on it behind `while (!searchServiceIsLoaded)`, so the parsing spinner stays up for good, and the .catch already written for that path in index.js could not run because nothing ever rejected. Signed-off-by: ppcvote <risky9763@gmail.com>
Signed-off-by: ppcvote <risky9763@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of what has changed
TableWrapper.bulkPutran its work insidenew Promise(async (resolve) => ...)with noreject, and the chunk write atindexed-db-wrapper.js:60was unguarded. When Dexie rejects there (QuotaExceededError, DatabaseClosedError, an aborted transaction) the rejection settled the executor's own throwaway promise, so the promise returned to the caller never settled at all. It hung rather than rejected.That matters because
SearchService.initializeAsyncawaits it on the cold-cache path, andindex.js:162then spins onwhile (!searchServiceIsLoaded)with the parsing spinner shown. The.catchalready written for that path atindex.js:139could not run, because nothing ever rejected.The fix takes
rejectand wraps the chunk loop in try/catch, following the shape already used inbackupSearchIndex. Droppingasyncfrom the executor also clearsno-async-promise-executor, which this repo's eslint config sets to error: one problem at line 26 before, none after.Two tests added to the existing file. They race the call against a 1s sentinel so a hang is distinguishable from a rejection; a plain
rejectsassertion would just time out and read as a slow test. Both reportReceived: "HUNG"before the change and pass after. Full suite: 44 passing.CHANGELOG.mdhas anUnreleasedsection for this. There was no pending heading, so please fold it into whichever version you cut next.Issues addressed by pull request
None open that I could find; the failure is silent, so it would surface as "search never finishes loading" rather than an error.
@isaisabel for review, per the template.