Skip to content

fix: Rip out continuations as they never really worked anyway#1936

Closed
marcelklehr wants to merge 6 commits intodevelopfrom
fix/rip-out-continuations
Closed

fix: Rip out continuations as they never really worked anyway#1936
marcelklehr wants to merge 6 commits intodevelopfrom
fix/rip-out-continuations

Conversation

@marcelklehr
Copy link
Member

No description provided.

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr marcelklehr requested a review from Copilot May 2, 2025 11:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the continuation mechanism from the sync process and makes several related adjustments across tests and adapter implementations. Key changes include:

  • Disabling tests for fuzzed sync operations via it.skip and replacing calls to syncAccountWithInterrupts with direct account.sync() calls.
  • Updating the sync process in strategies and adapters to handle cancellation and error reporting without continuations.
  • Removing the continuation-related API from AccountStorage and Account classes.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/test/test.js Disabled some sync tests and replaced interrupt logic with direct sync calls.
src/lib/strategies/Default.ts Updated error handling in the sync process with cancellation checks.
src/lib/native/NativeController.js Added logging and adjusted reset behavior for stuck syncing accounts.
src/lib/interfaces/AccountStorage.ts Removed unused continuation methods from the interface.
src/lib/browser/BrowserController.js Duplicated reset logic for stuck syncing accounts added with Logger logging.
src/lib/adapters/NextcloudBookmarks.ts & Linkwarden.ts Added cancellation checks before queuing network requests.
src/lib/Account.ts Removed all continuation handling and persistence logic in the sync process.
Comments suppressed due to low confidence (3)

src/test/test.js:6700

  • The test has been disabled using it.skip. Please ensure that temporarily skipping these tests is documented and tracked to avoid accidental loss of test coverage.
it.skip('should handle fuzzed changes with deletions from two clients with interrupts' + (ACCOUNT_DATA.type === 'fake' ? ' (with caching)' : ''), interruptBenchmark = async function() {

src/lib/interfaces/AccountStorage.ts:31

  • Since the continuation methods have been removed, update the interface documentation to reflect their removal and ensure that any related documentation is cleaned up.
getCurrentContinuation(): Promise<ISerializedSyncProcess|null>;

src/lib/Account.ts:298

  • With the removal of the continuation mechanism, confirm that all references to continuation persistence have been fully removed and that the new sync process does not depend on this state.
await this.storage.setCurrentContinuation(null)

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
and secure it against errors
and fix matchAllErrors

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
Copy link
Member Author

Probably not a good idea.

@marcelklehr marcelklehr closed this May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants