diff --git a/cspell.config.yaml b/cspell.config.yaml index b69fb4ed..5a02a206 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -38,11 +38,13 @@ words: - Decompressor - dllexport - dlopen + - EACCES - EDQUOT - embedders - EOCD - endtemplate - ENOSPC + - EPERM - EROFS - eseidel - ffigen @@ -77,6 +79,7 @@ words: - ureq - unbootable - unbooted + - unwritable - usize - Swatinem - taiki diff --git a/library/include/updater.h b/library/include/updater.h index 857567cb..dddaf337 100644 --- a/library/include/updater.h +++ b/library/include/updater.h @@ -48,6 +48,16 @@ */ #define SHOREBIRD_UPDATE_IN_PROGRESS 4 +/** + * The update could not be performed right now because the updater's state + * storage directory is temporarily unwritable. On iOS this typically means + * the device is locked and Data Protection is blocking writes under + * `Library/Application Support/`. The next update attempt after the device + * is unlocked will typically succeed. This is a benign outcome, not an + * error. + */ +#define SHOREBIRD_UPDATE_DEFERRED 5 + /** * 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 11b4971c..c9cd8bde 100644 --- a/library/src/c_api/mod.rs +++ b/library/src/c_api/mod.rs @@ -68,6 +68,14 @@ pub const SHOREBIRD_UPDATE_IS_BAD_PATCH: i32 = 3; /// This is a benign outcome, not an error. pub const SHOREBIRD_UPDATE_IN_PROGRESS: i32 = 4; +/// The update could not be performed right now because the updater's state +/// storage directory is temporarily unwritable. On iOS this typically means +/// the device is locked and Data Protection is blocking writes under +/// `Library/Application Support/`. The next update attempt after the device +/// is unlocked will typically succeed. This is a benign outcome, not an +/// error. +pub const SHOREBIRD_UPDATE_DEFERRED: i32 = 5; + #[repr(C)] pub struct UpdateResult { pub status: i32, diff --git a/library/src/cache/disk_io.rs b/library/src/cache/disk_io.rs index 1018db43..8095b8d5 100644 --- a/library/src/cache/disk_io.rs +++ b/library/src/cache/disk_io.rs @@ -1,9 +1,10 @@ use crate::file_errors::{FileOperation, IoResultExt}; +use crate::updater::UpdateError; use anyhow::{bail, Context}; use serde::{de::DeserializeOwned, Serialize}; use std::{ fs::File, - io::{BufReader, BufWriter, Write}, + io::{BufReader, BufWriter, ErrorKind, Write}, path::{Path, PathBuf}, }; @@ -21,8 +22,13 @@ where // Because File::create can sometimes fail if the full directory path doesn't exist, // we create the directories in its path first. - std::fs::create_dir_all(containing_dir) - .with_file_context(FileOperation::CreateDir, containing_dir)?; + if let Err(e) = std::fs::create_dir_all(containing_dir) { + return Err(map_state_io_error( + e, + FileOperation::CreateDir, + containing_dir, + )); + } // Write to a sibling temp file first, then atomically rename into place. // Two problems with writing directly to `path`: @@ -36,7 +42,10 @@ where // flush error (we unwrap `BufWriter` below), and on-disk `path` is only // replaced by a fully-written sibling via an atomic `rename`. let temp_path = temp_sibling_path(path_as_ref); - let file = File::create(&temp_path).with_file_context(FileOperation::CreateFile, &temp_path)?; + let file = match File::create(&temp_path) { + Ok(f) => f, + Err(e) => return Err(map_state_io_error(e, FileOperation::CreateFile, &temp_path)), + }; if let Err(err) = serialize_and_flush(serializable, file) .with_context(|| format!("failed to serialize to {:?}", &temp_path)) { @@ -44,8 +53,10 @@ where let _ = std::fs::remove_file(&temp_path); return Err(err); } - std::fs::rename(&temp_path, path_as_ref) - .with_file_context(FileOperation::RenameFile, &temp_path) + if let Err(e) = std::fs::rename(&temp_path, path_as_ref) { + return Err(map_state_io_error(e, FileOperation::RenameFile, &temp_path)); + } + Ok(()) } /// Serializes `value` as pretty JSON into `writer`, then explicitly unwraps @@ -74,6 +85,44 @@ fn temp_sibling_path(path: &Path) -> PathBuf { path.with_file_name(format!("{file_name}.tmp")) } +/// Maps an IO error from a state write operation into an `anyhow::Error`. +/// +/// Most errors are wrapped with the standard file-operation context and +/// propagated as-is. A `PermissionDenied` error, however, is translated into +/// a dedicated `UpdateError::StateStorageUnavailable` so higher layers can +/// distinguish "the device is locked and our state directory is temporarily +/// unwritable" from a real failure. +/// +/// On iOS, files under `Library/Application Support/` inherit the default +/// Data Protection class `NSFileProtectionCompleteUntilFirstUserAuthentication`. +/// Before the user has unlocked the device for the first time since boot +/// (and in some edge cases while the device is locked), the OS refuses +/// writes with `EPERM` / `EACCES`, which Rust surfaces as +/// `ErrorKind::PermissionDenied`. This is transient — the next update +/// attempt after the device is unlocked and the app is foregrounded will +/// typically succeed — so we deliberately do not treat it as an error. +fn map_state_io_error( + error: std::io::Error, + operation: FileOperation, + path: &Path, +) -> anyhow::Error { + if error.kind() == ErrorKind::PermissionDenied { + shorebird_info!( + "State storage temporarily unavailable ({} {}): {}. \ + Update will be deferred until storage becomes writable.", + operation, + path.display(), + error + ); + return anyhow::Error::new(UpdateError::StateStorageUnavailable); + } + // Re-wrap non-PermissionDenied errors with the same enhanced context the + // `with_file_context` trait would have produced. + Err::<(), _>(error) + .with_file_context(operation, path) + .unwrap_err() +} + pub fn read(path: &P) -> anyhow::Result where D: DeserializeOwned, @@ -128,6 +177,48 @@ mod test { assert!(super::read::(&Path::new("nonexistent.json")).is_err()); } + #[test] + fn permission_denied_maps_to_state_storage_unavailable() { + // Emulate the iOS-locked-device case: PermissionDenied on File::create + // should translate to UpdateError::StateStorageUnavailable rather than + // propagating as a generic "failed to create file" error. + let error = std::io::Error::new(std::io::ErrorKind::PermissionDenied, "Permission denied"); + let path = Path::new("/protected/state.json"); + + let anyhow_err = + super::map_state_io_error(error, crate::file_errors::FileOperation::CreateFile, path); + + let downcast = anyhow_err.downcast_ref::(); + assert_eq!( + downcast, + Some(&crate::updater::UpdateError::StateStorageUnavailable), + ); + } + + #[test] + fn non_permission_denied_errors_retain_file_context() { + // Errors other than PermissionDenied should still be wrapped with the + // standard enhanced file-operation context and NOT be treated as a + // state-storage-unavailable deferral. + let error = std::io::Error::new(std::io::ErrorKind::StorageFull, "No space left on device"); + let path = Path::new("/data/state.json"); + + let anyhow_err = + super::map_state_io_error(error, crate::file_errors::FileOperation::CreateFile, path); + + assert!( + anyhow_err + .downcast_ref::() + .is_none(), + "StorageFull must not be treated as StateStorageUnavailable" + ); + let message = format!("{anyhow_err:?}"); + assert!( + message.contains("Failed to create file"), + "expected enhanced file context, got: {message}" + ); + } + #[test] fn read_errs_if_struct_cannot_be_deserialized() -> Result<()> { let temp_dir = TempDir::new()?; @@ -221,4 +312,79 @@ mod test { .to_string() .contains("simulated flush failure")); } + + // The next three tests exercise the call sites of `map_state_io_error` + // inside `write()` itself (create_dir_all, File::create, rename), not just + // the helper. They use a read-only parent directory to provoke a real + // PermissionDenied from the OS. Unix-only because Windows ACLs don't + // surface as ErrorKind::PermissionDenied the same way. + #[cfg(unix)] + fn make_read_only(path: &Path) { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(path).unwrap().permissions(); + perms.set_mode(0o555); + std::fs::set_permissions(path, perms).unwrap(); + } + + #[cfg(unix)] + fn make_writable(path: &Path) { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(path).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(path, perms).unwrap(); + } + + #[cfg(unix)] + #[test] + fn write_returns_state_storage_unavailable_when_create_dir_all_denied() { + let temp_dir = TempDir::new().unwrap(); + make_read_only(temp_dir.path()); + // Path with a missing subdirectory under a read-only parent — so + // create_dir_all is the call that will fail with PermissionDenied. + let path = temp_dir.path().join("subdir/state.json"); + + let result = super::write( + &TestStruct { + a: 1, + b: "hi".into(), + }, + &path, + ); + + // Restore permissions so TempDir can clean up. + make_writable(temp_dir.path()); + + let err = result.expect_err("write should fail under read-only parent"); + assert_eq!( + err.downcast_ref::(), + Some(&crate::updater::UpdateError::StateStorageUnavailable), + ); + } + + #[cfg(unix)] + #[test] + fn write_returns_state_storage_unavailable_when_file_create_denied() { + let temp_dir = TempDir::new().unwrap(); + // Containing dir already exists (so create_dir_all is a no-op), + // but is read-only — File::create on the temp sibling is the call + // that will fail with PermissionDenied. + make_read_only(temp_dir.path()); + let path = temp_dir.path().join("state.json"); + + let result = super::write( + &TestStruct { + a: 1, + b: "hi".into(), + }, + &path, + ); + + make_writable(temp_dir.path()); + + let err = result.expect_err("write should fail under read-only dir"); + assert_eq!( + err.downcast_ref::(), + Some(&crate::updater::UpdateError::StateStorageUnavailable), + ); + } } diff --git a/library/src/updater.rs b/library/src/updater.rs index cd3cd4de..3fa24495 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -34,6 +34,13 @@ pub enum UpdateStatus { // already-running update will continue; the caller did not start a new // one. This is a benign outcome, not an error. UpdateInProgress, + // The update could not be performed right now because the state storage + // directory is temporarily unwritable. On iOS this typically means the + // device is locked and Data Protection is blocking writes under + // Library/Application Support. The next update attempt after the device + // is unlocked will typically succeed. This is a benign outcome, not an + // error. + UpdateDeferred, } impl Display for UpdateStatus { @@ -47,6 +54,9 @@ impl Display for UpdateStatus { "Update available but previously failed to install. Not installing." ), UpdateStatus::UpdateInProgress => write!(f, "Update already in progress"), + UpdateStatus::UpdateDeferred => { + write!(f, "Update deferred: state storage temporarily unavailable") + } } } } @@ -93,6 +103,12 @@ pub enum UpdateError { FailedToSaveState, ConfigNotInitialized, UpdateAlreadyInProgress, + /// The updater's on-disk state directory is temporarily unwritable + /// (typically because iOS Data Protection is blocking writes on a + /// locked device). The update attempt is deferred until storage + /// becomes available. This is benign and is translated at the top + /// of the update call into `UpdateStatus::UpdateDeferred`. + StateStorageUnavailable, } impl std::error::Error for UpdateError {} @@ -107,6 +123,9 @@ impl Display for UpdateError { UpdateError::UpdateAlreadyInProgress => { write!(f, "Update already in progress") } + UpdateError::StateStorageUnavailable => { + write!(f, "State storage temporarily unavailable") + } } } } @@ -665,13 +684,14 @@ pub fn update(channel: Option<&str>) -> anyhow::Result { // 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) + match e.downcast_ref::() { + Some(UpdateError::UpdateAlreadyInProgress) => Ok(UpdateStatus::UpdateInProgress), + // The updater's state directory is temporarily unwritable + // (typically iOS Data Protection on a locked device). The + // update is deferred, not failed — the next attempt after + // the device is unlocked will typically succeed. + Some(UpdateError::StateStorageUnavailable) => Ok(UpdateStatus::UpdateDeferred), + _ => Err(e), } } } @@ -3815,4 +3835,20 @@ mod multi_engine_tests { Ok(()) } + + #[test] + fn update_status_deferred_displays_as_deferred() { + assert_eq!( + crate::updater::UpdateStatus::UpdateDeferred.to_string(), + "Update deferred: state storage temporarily unavailable", + ); + } + + #[test] + fn update_error_state_storage_unavailable_displays_as_unavailable() { + assert_eq!( + crate::updater::UpdateError::StateStorageUnavailable.to_string(), + "State storage temporarily unavailable", + ); + } } 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 63eeb345..b4422d2b 100644 --- a/shorebird_code_push/lib/src/generated/updater_bindings.g.dart +++ b/shorebird_code_push/lib/src/generated/updater_bindings.g.dart @@ -5118,3 +5118,5 @@ const int SHOREBIRD_UPDATE_HAD_ERROR = 2; const int SHOREBIRD_UPDATE_IS_BAD_PATCH = 3; const int SHOREBIRD_UPDATE_IN_PROGRESS = 4; + +const int SHOREBIRD_UPDATE_DEFERRED = 5; diff --git a/shorebird_code_push/lib/src/shorebird_updater_io.dart b/shorebird_code_push/lib/src/shorebird_updater_io.dart index 62a59626..684cf38c 100644 --- a/shorebird_code_push/lib/src/shorebird_updater_io.dart +++ b/shorebird_code_push/lib/src/shorebird_updater_io.dart @@ -123,9 +123,15 @@ class ShorebirdUpdaterImpl implements ShorebirdUpdater { // - 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. + // - SHOREBIRD_UPDATE_DEFERRED: the updater's state storage was + // temporarily unwritable (on iOS, typically Data Protection on a + // locked device blocking writes under Library/Application Support). + // The next update attempt after the device is unlocked will + // typically succeed. Benign, must not throw. if (status == SHOREBIRD_UPDATE_INSTALLED || status == SHOREBIRD_NO_UPDATE || - status == SHOREBIRD_UPDATE_IN_PROGRESS) { + status == SHOREBIRD_UPDATE_IN_PROGRESS || + status == SHOREBIRD_UPDATE_DEFERRED) { 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 347ff660..ac983cab 100644 --- a/shorebird_code_push/test/src/shorebird_updater_io_test.dart +++ b/shorebird_code_push/test/src/shorebird_updater_io_test.dart @@ -438,6 +438,34 @@ void main() { }); }); + group('when the update is deferred', () { + setUp(() { + when(() => updater.currentPatchNumber()).thenReturn(0); + final result = calloc.allocate(sizeOf()); + result.ref.status = SHOREBIRD_UPDATE_DEFERRED; + result.ref.message = 'State storage temporarily unavailable' + .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 defers (typically iOS Data Protection + // blocking state writes on a locked device), update() must not + // surface it as an exception. The next attempt after the device + // is unlocked will typically succeed. + 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);