Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

sync: move try-catch out of needsSyncing and handle errors in fullSync #207

Merged
merged 1 commit into from
Nov 23, 2020

Conversation

fredrikekre
Copy link
Contributor

@fredrikekre fredrikekre commented Nov 20, 2020

The motivation for this is bitwarden/cli#129
where failed sync's are swallowed by try-catch. By moving the try-catch
to the outside it is possible to reuse the already existing
allowThrowOnError argument which callers can use to signal whether
fullSync should throw or ignore errors silently. This patch is
companioned with a patch to the SyncCommand CLI command to pass
allowThrowOnError.

Edit: see bitwarden/cli#190, although the patches are independet of each other.

@CLAassistant
Copy link

CLAassistant commented Nov 20, 2020

CLA assistant check
All committers have signed the CLA.

@fredrikekre fredrikekre changed the title sync: move try-catch out of needsSyncing and handle errors it in fullSync sync: move try-catch out of needsSyncing and handle errors in fullSync Nov 20, 2020
Copy link
Contributor

@cscharf cscharf left a comment

Choose a reason for hiding this comment

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

Thanks for your submission! Looks good, simplified; please see 1 small formatting nit.

src/services/sync.service.ts Outdated Show resolved Hide resolved
…Sync

The motivation for this is bitwarden/cli#129
where failed sync's are swallowed by try-catch. By moving the try-catch
to the outside it is possible to reuse the already existing
allowThrowOnError argument which callers can use to signal whether
fullSync should throw or ignore errors silently. This patch is
companioned with a patch to the SyncCommand CLI command to pass
allowThrowOnError.
Copy link
Contributor

@cscharf cscharf left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@cscharf cscharf merged commit adcc618 into bitwarden:master Nov 23, 2020
@fredrikekre fredrikekre deleted the fe/sync branch November 23, 2020 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants