diff --git a/library/include/updater.h b/library/include/updater.h index df1785c..857567c 100644 --- a/library/include/updater.h +++ b/library/include/updater.h @@ -41,6 +41,13 @@ */ #define SHOREBIRD_UPDATE_IS_BAD_PATCH 3 +/** + * Another update was already in progress when this call was made. The + * already-running update will continue; the caller did not start a new one. + * This is a benign outcome, not an error. + */ +#define SHOREBIRD_UPDATE_IN_PROGRESS 4 + /** * Struct containing configuration parameters for the updater. * Passed to all updater functions. diff --git a/library/src/c_api/mod.rs b/library/src/c_api/mod.rs index 13e15bd..11b4971 100644 --- a/library/src/c_api/mod.rs +++ b/library/src/c_api/mod.rs @@ -63,6 +63,11 @@ pub const SHOREBIRD_UPDATE_HAD_ERROR: i32 = 2; /// The downloaded patch was not installed because it was invalid. pub const SHOREBIRD_UPDATE_IS_BAD_PATCH: i32 = 3; +/// Another update was already in progress when this call was made. The +/// already-running update will continue; the caller did not start a new one. +/// This is a benign outcome, not an error. +pub const SHOREBIRD_UPDATE_IN_PROGRESS: i32 = 4; + #[repr(C)] pub struct UpdateResult { pub status: i32, @@ -1007,7 +1012,13 @@ mod test { shorebird_start_update_thread(); // Wait for the thread to start. std::thread::sleep(std::time::Duration::from_millis(100)); - assert!(updater::update(None).is_err()); + // When another update is already in progress, `update()` returns + // `UpdateStatus::UpdateInProgress` rather than surfacing an error. + // The in-flight update continues on its own. + assert_eq!( + updater::update(None).unwrap(), + crate::UpdateStatus::UpdateInProgress + ); } // Unlock the lock, and wait for the thread to finish. std::thread::sleep(std::time::Duration::from_millis(100)); diff --git a/library/src/updater.rs b/library/src/updater.rs index 2f660e3..b280f1b 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -30,6 +30,10 @@ pub enum UpdateStatus { UpdateInstalled, UpdateHadError, UpdateIsBadPatch, + // Another update was already in progress when this call was made. The + // already-running update will continue; the caller did not start a new + // one. This is a benign outcome, not an error. + UpdateInProgress, } impl Display for UpdateStatus { @@ -42,6 +46,7 @@ impl Display for UpdateStatus { f, "Update available but previously failed to install. Not installing." ), + UpdateStatus::UpdateInProgress => write!(f, "Update already in progress"), } } } @@ -651,7 +656,23 @@ fn cleanup_download_artifacts(download_path: &Path) { /// Synchronously checks for an update and downloads and installs it if available. pub fn update(channel: Option<&str>) -> anyhow::Result { - with_updater_thread_lock(|lock_state| update_internal(lock_state, channel)) + match with_updater_thread_lock(|lock_state| update_internal(lock_state, channel)) { + Ok(status) => Ok(status), + Err(e) => { + // "Another update is already running" is a benign outcome — the + // in-progress update (typically the automatic updater thread) will + // continue on its own. Surface it as a non-error status so callers + // that monitor `update()` exceptions do not see it as a failure. + if matches!( + e.downcast_ref::(), + Some(UpdateError::UpdateAlreadyInProgress) + ) { + Ok(UpdateStatus::UpdateInProgress) + } else { + Err(e) + } + } + } } /// The first 4 bytes of any zstd compressed frame. diff --git a/shorebird_code_push/lib/src/generated/updater_bindings.g.dart b/shorebird_code_push/lib/src/generated/updater_bindings.g.dart index 1b78aa6..63eeb34 100644 --- a/shorebird_code_push/lib/src/generated/updater_bindings.g.dart +++ b/shorebird_code_push/lib/src/generated/updater_bindings.g.dart @@ -5116,3 +5116,5 @@ const int SHOREBIRD_UPDATE_INSTALLED = 1; const int SHOREBIRD_UPDATE_HAD_ERROR = 2; const int SHOREBIRD_UPDATE_IS_BAD_PATCH = 3; + +const int SHOREBIRD_UPDATE_IN_PROGRESS = 4; diff --git a/shorebird_code_push/lib/src/shorebird_updater_io.dart b/shorebird_code_push/lib/src/shorebird_updater_io.dart index 35967f1..62a5962 100644 --- a/shorebird_code_push/lib/src/shorebird_updater_io.dart +++ b/shorebird_code_push/lib/src/shorebird_updater_io.dart @@ -117,12 +117,15 @@ class ShorebirdUpdaterImpl implements ShorebirdUpdater { final status = result.ref.status; - // SHOREBIRD_UPDATE_INSTALLED is the success case. SHOREBIRD_NO_UPDATE - // (the app is already up to date) is also a successful outcome of a call - // to update() and must not throw — previously it surfaced as a confusing - // `UpdateException: No update (noUpdate)` in customer telemetry. + // Successful outcomes of update(): + // - SHOREBIRD_UPDATE_INSTALLED: a new patch was downloaded and installed. + // - SHOREBIRD_NO_UPDATE: the app is already running the latest patch. + // - SHOREBIRD_UPDATE_IN_PROGRESS: another update (typically the automatic + // updater thread) was already running; the caller did not start a new + // one. This is benign and must not surface as an exception. if (status == SHOREBIRD_UPDATE_INSTALLED || - status == SHOREBIRD_NO_UPDATE) { + status == SHOREBIRD_NO_UPDATE || + status == SHOREBIRD_UPDATE_IN_PROGRESS) { return; } diff --git a/shorebird_code_push/test/src/shorebird_updater_io_test.dart b/shorebird_code_push/test/src/shorebird_updater_io_test.dart index 57aeef4..347ff66 100644 --- a/shorebird_code_push/test/src/shorebird_updater_io_test.dart +++ b/shorebird_code_push/test/src/shorebird_updater_io_test.dart @@ -411,6 +411,33 @@ void main() { }); }); + group('when another update is already in progress', () { + setUp(() { + when(() => updater.currentPatchNumber()).thenReturn(0); + final result = calloc.allocate(sizeOf()); + result.ref.status = SHOREBIRD_UPDATE_IN_PROGRESS; + result.ref.message = + 'Update already in progress'.toNativeUtf8().cast(); + addTearDown(() { + calloc + ..free(result.ref.message) + ..free(result); + }); + when(() => updater.update()).thenReturn(result); + shorebirdUpdater = ShorebirdUpdaterImpl(updater: updater, run: run); + }); + + test('returns normally and does not throw', () async { + // When the Rust updater reports that another update is already + // running, `update()` must not surface it as an exception — it is + // a benign outcome. The already-running update continues on its + // own; the caller simply did not start a new one. + await expectLater(shorebirdUpdater.update(), completes); + verify(updater.update).called(1); + verify(() => updater.freeUpdateResult(any())).called(1); + }); + }); + group('when an error occurs during download', () { setUp(() { when(() => updater.currentPatchNumber()).thenReturn(0);