flutter/shorebird-updater
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: report launch failure when a patch was in the process of booting on app start (#189)

Browse Source 
* report launch failure when a patch was in the process of booting on app start

* refactor: extract shared fake patch test logic into function

* cleanup

* remove commented-out code

* cleanup

* Cleanup

* cleanup

* rename

* update comments

* add todo

* remove added logs

* update comments

* make patch_event a proper constructor

* Split with_mut_state out from with_state

* update comment

* feat: track known_bad_patches instead of highest_seen_patch to support rollbacks (#191)

* feat: track known_bad_patches instead of highest_seen_patch to support rollbacks

* docs

* do not send patch number, do not install already installed patches

* remove patch number from PatchCheckRequest

* tests

* more tests

* fix merge issues
This commit is contained in:
Bryan Oltman
2024-07-19 17:41:04 -04:00
committed by GitHub
parent 6c36ab68b9
commit 6ba524716c
5 changed files with 336 additions and 301 deletions
Download Patch File Download Diff File Expand all files Collapse all files
library/src/cache/patch_manager.rs
Vendored
+47 -170
View File
@@ -2,7 +2,10 @@ use super::{disk_io, signing, PatchInfo};
use anyhow::{bail, Context, Result};
use core::fmt::Debug;
use serde::{Deserialize, Serialize};
use std::path::{Path, PathBuf};
use std::{
collections::HashSet,
path::{Path, PathBuf},
};
#[cfg(test)]
use mockall::automock;
@@ -50,18 +53,19 @@ struct PatchesState {
/// - the system initializes (on_init, we take this to mean the patch failed to boot)
currently_booting_patch: Option<PatchMetadata>,
/// The highest patch number we have seen. This may be higher than the last booted
/// patch or next patch if we downloaded a patch that failed to boot.
highest_seen_patch_number: Option<usize>,
/// A list of patch numbers that we have tried and failed to install.
/// We should never attempt to download or install these again for the
/// current release.
known_bad_patches: HashSet<usize>,
}
/// Abstracts the process of managing patches.
/// Abstracts the storage of patches on disk.
///
/// The impementation of this (PatchManager) should only be responsible for translating what is on
/// disk into a form that is useful for the updater and vice versa. Some business logic has crept in
/// in the form of validation, and we should consider moving that into a separate module.
#[cfg_attr(test, automock)]
pub trait ManagePatches {
/// Triggers any initialization logic needed by the patch manager. This is intended
/// to be called when Shorebird is initialized by the Flutter engine (shorebird_init).
fn on_init(&mut self) -> Result<()>;
/// Copies the patch file at file_path to the manager's directory structure sets
/// this patch as the next patch to boot.
///
@@ -103,9 +107,8 @@ pub trait ManagePatches {
/// that it will never be returned as the next boot or last booted patch.
fn record_boot_failure_for_patch(&mut self, patch_number: usize) -> Result<()>;
/// The highest patch number that has been added. This may be higher than the
/// last booted or next boot patch if we downloaded a patch that failed to boot.
fn highest_seen_patch_number(&self) -> Option<usize>;
/// Whether we have failed to boot from the patch with `patch_number`.
fn is_known_bad_patch(&self, patch_number: usize) -> bool;
/// Resets the patch manager to its initial state, removing all patches. This is
/// intended to be used when a new release version is installed.
@@ -256,6 +259,8 @@ impl PatchManager {
/// successfully booted patch. If the last successfully booted patch is not bootable or has the same number
/// as the patch we're falling back from, we clear it as well.
fn try_fall_back_from_patch(&mut self, bad_patch_number: usize) {
// Continue even if we fail to delete the patch artifacts. It's more important to not try to
// boot from a bad patch than to delete its artifacts.
// No need to log failure – delete_patch_artifacts logs for us.
let _ = self.delete_patch_artifacts(bad_patch_number);
@@ -316,19 +321,6 @@ impl PatchManager {
}
impl ManagePatches for PatchManager {
fn on_init(&mut self) -> Result<()> {
// If we were booting a patch but never recorded a successful boot, we assume that
// the patch failed to boot. Attempt to fall back.
// TODO: this should record a PatchInstallFailure event. https://github.com/shorebirdtech/updater/issues/188
if let Some(failed_boot_patch) = self.patches_state.currently_booting_patch.clone() {
self.try_fall_back_from_patch(failed_boot_patch.number);
self.patches_state.currently_booting_patch = None;
self.save_patches_state()?;
}
Ok(())
}
// The explicit lifetime is required for automock to work with Options.
// See https://github.com/asomers/mockall/issues/61.
#[allow(clippy::needless_lifetimes)]
@@ -369,11 +361,6 @@ impl ManagePatches for PatchManager {
}
self.patches_state.next_boot_patch = Some(new_patch);
self.patches_state.highest_seen_patch_number = self
.patches_state
.highest_seen_patch_number
.map(|highest_patch_number: usize| highest_patch_number.max(patch_number))
.or(Some(patch_number));
self.save_patches_state()
}
@@ -452,12 +439,13 @@ impl ManagePatches for PatchManager {
fn record_boot_failure_for_patch(&mut self, patch_number: usize) -> Result<()> {
self.patches_state.currently_booting_patch = None;
self.patches_state.known_bad_patches.insert(patch_number);
self.try_fall_back_from_patch(patch_number);
self.save_patches_state()
}
fn highest_seen_patch_number(&self) -> Option<usize> {
self.patches_state.highest_seen_patch_number
fn is_known_bad_patch(&self, patch_number: usize) -> bool {
self.patches_state.known_bad_patches.contains(&patch_number)
}
fn reset(&mut self) -> Result<()> {
@@ -523,55 +511,13 @@ mod debug_tests {
let temp_dir = TempDir::new("patch_manager").unwrap();
let patch_manager = PatchManager::new(temp_dir.path().to_owned(), Some("public_key"));
let expected_str = format!(
"PatchManager {{ root_dir: \"{}\", patches_state: PatchesState {{ last_booted_patch: None, next_boot_patch: None, currently_booting_patch: None, highest_seen_patch_number: None }}, patch_public_key: Some(\"public_key\") }}",
"PatchManager {{ root_dir: \"{}\", patches_state: PatchesState {{ last_booted_patch: None, next_boot_patch: None, currently_booting_patch: None, known_bad_patches: {{}} }}, patch_public_key: Some(\"public_key\") }}",
temp_dir.path().display()
);
assert_eq!(format!("{:?}", patch_manager), expected_str);
}
}
#[cfg(test)]
mod on_init_tests {
use super::*;
#[test]
fn clears_currently_booting_patch() -> Result<()> {
let temp_dir = TempDir::new("patch_manager").unwrap();
let mut manager = PatchManager::manager_for_test(&temp_dir);
// Add a patch and start to boot from it.
manager.add_patch_for_test(&temp_dir, 1)?;
manager.record_boot_start_for_patch(1)?;
assert_eq!(
manager
.patches_state
.currently_booting_patch
.as_ref()
.map(|p| p.number),
Some(1)
);
assert_eq!(
manager.next_boot_patch().as_ref().map(|p| p.number),
Some(1)
);
// Simulate that the app is being started fresh (e.g. from a crash)
manager = PatchManager::manager_for_test(&temp_dir);
// Ensure that we didn't somehow lose next_boot_patch when recreating the manager.
assert_eq!(
manager.next_boot_patch().as_ref().map(|p| p.number),
Some(1)
);
manager.on_init()?;
// Verify that we are no longer booting from patch 1.
assert!(manager.next_boot_patch().is_none());
Ok(())
}
}
#[cfg(test)]
mod add_patch_tests {
use super::*;
@@ -620,36 +566,6 @@ mod add_patch_tests {
})
);
assert!(!file_path.exists());
assert_eq!(manager.highest_seen_patch_number(), Some(patch_number));
}
#[test]
fn does_not_set_higher_highest_seen_patch_number_if_added_patch_is_lower() -> Result<()> {
let patch_file_contents = "patch contents";
let temp_dir = TempDir::new("patch_manager")?;
let mut manager = PatchManager::manager_for_test(&temp_dir);
assert!(manager.highest_seen_patch_number().is_none());
// Add patch 1
let file_path = &temp_dir.path().join("patch.vmcode");
std::fs::write(file_path, patch_file_contents)?;
assert!(manager.add_patch(1, file_path, "hash", None).is_ok());
assert_eq!(manager.highest_seen_patch_number(), Some(1));
// Add patch 4, expect 4 to be the highest patch number we've seen
let file_path = &temp_dir.path().join("patch.vmcode");
std::fs::write(file_path, patch_file_contents)?;
assert!(manager.add_patch(4, file_path, "hash", None).is_ok());
assert_eq!(manager.highest_seen_patch_number(), Some(4));
// Add patch 3, expect 4 to still be the highest patch number we've seen
let file_path = &temp_dir.path().join("patch.vmcode");
std::fs::write(file_path, patch_file_contents)?;
assert!(manager.add_patch(3, file_path, "hash", None).is_ok());
assert_eq!(manager.highest_seen_patch_number(), Some(4));
Ok(())
}
}
@@ -758,6 +674,7 @@ mod next_boot_patch_tests {
assert!(manager.add_patch(1, file_path, "hash", None).is_ok());
assert!(manager.record_boot_start_for_patch(1).is_ok());
assert!(manager.record_boot_success().is_ok());
assert!(!manager.is_known_bad_patch(1));
// Add patch 2, pretend it failed to boot.
let file_path = &temp_dir.path().join("patch2.vmcode");
@@ -765,6 +682,7 @@ mod next_boot_patch_tests {
assert!(manager.add_patch(2, file_path, "hash", None).is_ok());
assert!(manager.record_boot_start_for_patch(2).is_ok());
assert!(manager.record_boot_failure_for_patch(2).is_ok());
assert!(manager.is_known_bad_patch(2));
// Verify that we will next attempt to boot from patch 1.
assert_eq!(manager.next_boot_patch().unwrap().number, 1);
@@ -799,6 +717,13 @@ mod next_boot_patch_tests {
// Verify that we will not attempt to boot from either patch.
assert!(manager.next_boot_patch().is_none());
// Patch 1 should *not* be considered bad, as we successfully booted from it and it only
// became corrupted after that. Downloading it a second time might resolve the issue.
assert!(!manager.is_known_bad_patch(1));
// Patch 2 failed to boot, so it should be considered bad.
assert!(manager.is_known_bad_patch(2));
Ok(())
}
@@ -814,6 +739,7 @@ mod next_boot_patch_tests {
// Because there is no previous patch, we should not attempt to boot any patch.
assert!(manager.next_boot_patch().is_none());
assert!(manager.is_known_bad_patch(1));
Ok(())
}
@@ -835,49 +761,8 @@ mod next_boot_patch_tests {
// Verify that we will next attempt to boot from patch 1.
assert_eq!(manager.next_boot_patch().unwrap().number, 1);
Ok(())
}
#[test]
fn returns_none_if_first_patch_did_not_successfully_boot() -> Result<()> {
let temp_dir = TempDir::new("patch_manager")?;
let mut manager = PatchManager::manager_for_test(&temp_dir);
// Add a first patch and record that we started booting it, but not that it succeeded.
manager.add_patch_for_test(&temp_dir, 1)?;
manager.record_boot_start_for_patch(1)?;
// Simulate that the app is being started fresh (e.g. from a crash)
manager = PatchManager::manager_for_test(&temp_dir);
manager.on_init()?;
assert!(manager.next_boot_patch().is_none());
Ok(())
}
#[test]
fn returns_none_if_next_patch_did_not_successfully_boot() -> Result<()> {
let temp_dir = TempDir::new("patch_manager")?;
let mut manager = PatchManager::manager_for_test(&temp_dir);
// Add a first patch and pretend it booted successfully.
manager.add_patch_for_test(&temp_dir, 1)?;
manager.record_boot_start_for_patch(1)?;
manager.record_boot_success()?;
// Add a second patch and record that we started booting it, but not that it succeeded.
manager.add_patch_for_test(&temp_dir, 2)?;
manager.record_boot_start_for_patch(2)?;
// Simulate that the app is being started fresh (e.g. from a crash)
manager = PatchManager::manager_for_test(&temp_dir);
manager.on_init()?;
assert!(manager
.next_boot_patch()
.is_some_and(|patch| patch.number == 1));
assert!(!manager.is_known_bad_patch(1));
assert!(manager.is_known_bad_patch(2));
Ok(())
}
@@ -1081,7 +966,16 @@ mod fall_back_tests {
manager.try_fall_back_from_patch(1);
assert!(manager.patches_state.last_booted_patch.is_none());
assert_eq!(manager.patches_state.next_boot_patch.unwrap().number, 2);
assert_eq!(
manager
.patches_state
.next_boot_patch
.clone()
.unwrap()
.number,
2
);
assert!(manager.is_known_bad_patch(1));
Ok(())
}
@@ -1231,6 +1125,7 @@ mod record_boot_failure_for_patch_tests {
manager.add_patch_for_test(&temp_dir, 1)?;
assert!(manager.record_boot_start_for_patch(1).is_ok());
assert!(manager.record_boot_success().is_ok());
assert!(!manager.is_known_bad_patch(1));
let succeeded_patch_artifact_path = manager.patch_artifact_path(1);
manager.add_patch_for_test(&temp_dir, 2)?;
@@ -1243,6 +1138,7 @@ mod record_boot_failure_for_patch_tests {
assert!(manager.record_boot_start_for_patch(2).is_ok());
assert!(manager.record_boot_failure_for_patch(2).is_ok());
assert!(!failed_patch_artifact_path.exists());
assert!(manager.is_known_bad_patch(2));
Ok(())
}
@@ -1260,39 +1156,20 @@ mod record_boot_failure_for_patch_tests {
assert_eq!(manager.last_successfully_booted_patch().unwrap().number, 1);
assert_eq!(manager.next_boot_patch().unwrap().number, 1);
assert!(patch_artifact_path.exists());
assert!(!manager.is_known_bad_patch(1));
// Now pretend it failed to boot
assert!(manager.record_boot_start_for_patch(1).is_ok());
assert!(manager.record_boot_failure_for_patch(1).is_ok());
assert!(manager.last_successfully_booted_patch().is_none());
assert!(manager.next_boot_patch().is_none());
assert!(manager.is_known_bad_patch(1));
assert!(!patch_artifact_path.exists());
Ok(())
}
}
#[cfg(test)]
mod highest_seen_patch_number_tests {
use super::*;
use anyhow::{Ok, Result};
use tempdir::TempDir;
#[test]
fn returns_value_from_internal_state() -> Result<()> {
let temp_dir = TempDir::new("patch_manager")?;
let mut manager = PatchManager::manager_for_test(&temp_dir);
assert!(manager.patches_state.highest_seen_patch_number.is_none());
assert!(manager.highest_seen_patch_number().is_none());
manager.patches_state.highest_seen_patch_number = Some(1);
assert_eq!(manager.highest_seen_patch_number(), Some(1));
Ok(())
}
}
#[cfg(test)]
mod reset_tests {
use super::*;
library/src/cache/updater_state.rs
Vendored
+13 -19
View File
@@ -149,10 +149,6 @@ impl UpdaterState {
/// Patch management. All patch management is done via the patch manager.
impl UpdaterState {
pub fn on_init(&mut self) -> Result<()> {
self.patch_manager.on_init()
}
/// Records that we are attempting to boot the patch with patch_number.
pub fn record_boot_start_for_patch(&mut self, patch_number: usize) -> Result<()> {
self.patch_manager.record_boot_start_for_patch(patch_number)
@@ -203,15 +199,9 @@ impl UpdaterState {
.add_patch(patch.number, &patch.path, hash, signature)
}
/// Returns highest patch number that has been installed for this release.
/// This should represent the latest patch we still have on disk so as
/// to prevent re-downloading patches we already have.
/// This should essentially be the max of the patch number in the slots
/// and the bad patch list (we don't need to keep bad patches on disk
/// to know that they're bad).
/// Used by the patch check logic.
pub fn latest_seen_patch_number(&self) -> Option<usize> {
self.patch_manager.highest_seen_patch_number()
/// Returns true if we have previously failed to boot from patch `patch_number`.
pub fn is_known_bad_patch(&self, patch_number: usize) -> bool {
self.patch_manager.is_known_bad_patch(patch_number)
}
}
@@ -289,7 +279,6 @@ mod tests {
let mut next_version_state =
UpdaterState::load_or_new_on_error(&state.cache_dir, "1.0.0+2", None);
assert!(next_version_state.next_boot_patch().is_none());
assert!(next_version_state.latest_seen_patch_number().is_none());
}
#[test]
@@ -399,14 +388,19 @@ mod tests {
}
#[test]
fn latest_patch_number_returns_value_from_patch_manager() {
let highest_patch_number = 1;
fn is_known_bad_patch_returns_value_from_patch_manager() {
let tmp_dir = TempDir::new("example").unwrap();
let mut mock_manage_patches = MockManagePatches::new();
mock_manage_patches
.expect_highest_seen_patch_number()
.return_const(Some(highest_patch_number));
.expect_is_known_bad_patch()
.with(eq(1))
.return_const(true);
mock_manage_patches
.expect_is_known_bad_patch()
.with(eq(2))
.return_const(false);
let state = test_state(&tmp_dir, mock_manage_patches);
assert_eq!(state.latest_seen_patch_number(), Some(highest_patch_number));
assert!(state.is_known_bad_patch(1));
assert!(!state.is_known_bad_patch(2));
}
}
library/src/events.rs
+20
View File
@@ -3,6 +3,11 @@
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use crate::{
config::{current_arch, current_platform, UpdateConfig},
time,
};
#[derive(Debug, Clone, PartialEq)]
pub enum EventType {
PatchInstallSuccess,
@@ -62,3 +67,18 @@ pub struct PatchEvent {
/// When this event occurred as a Unix epoch timestamp in seconds.
pub timestamp: u64,
}
impl PatchEvent {
/// Creates a `PatchEvent` for the given `EventType` and patch number for reporting to the server.
pub fn new(config: &UpdateConfig, event_type: EventType, patch_number: usize) -> PatchEvent {
PatchEvent {
app_id: config.app_id.clone(),
arch: current_arch().to_string(),
identifier: event_type,
patch_number,
platform: current_platform().to_string(),
release_version: config.release_version.clone(),
timestamp: time::unix_timestamp(),
}
}
}
library/src/network.rs
+2 -5
View File
@@ -173,14 +173,12 @@ pub struct PatchCheckRequest {
/// running. Patches are keyed to release versions and will only be
/// offered to clients running the same release version.
pub release_version: String,
/// The latest patch number that the client has downloaded.
/// Not necessarily the one it's running (if some have been marked bad).
#[serde(skip_serializing_if = "Option::is_none")]
pub patch_number: Option<usize>,
/// Platform (e.g. "android", "ios", "windows", "macos", "linux").
pub platform: String,
/// Architecture we're running (e.g. "aarch64", "x86", "x86_64").
pub arch: String,
// We specifically do not send a patch number as part of this request because we always want to
// know what the latest available patch is.
}
/// The request body for the create patch install event endpoint.
@@ -290,7 +288,6 @@ mod tests {
app_id: "".to_string(),
channel: "".to_string(),
release_version: "".to_string(),
patch_number: None,
platform: "".to_string(),
arch: "".to_string(),
},
library/src/updater.rs
+254 -107
View File
@@ -5,8 +5,7 @@ use std::fs::{self};
use std::io::{Cursor, Read, Seek};
use std::path::{Path, PathBuf};
use anyhow::bail;
use anyhow::Context;
use anyhow::{bail, Context, Result};
use dyn_clone::DynClone;
use crate::cache::{PatchInfo, UpdaterState};
@@ -16,7 +15,6 @@ use crate::logging::init_logging;
use crate::network::{
download_to_path, patches_check_url, NetworkHooks, PatchCheckRequest, PatchCheckResponse,
};
use crate::time;
use crate::updater_lock::{with_updater_thread_lock, UpdaterLockState};
use crate::yaml::YamlConfig;
@@ -30,10 +28,12 @@ pub use crate::config::testing_reset_config;
#[cfg(test)]
pub use crate::network::{DownloadFileFn, Patch, PatchCheckRequestFn};
#[derive(Debug, PartialEq)]
pub enum UpdateStatus {
NoUpdate,
UpdateInstalled,
UpdateHadError,
UpdateIsBadPatch,
}
impl Display for UpdateStatus {
@@ -42,15 +42,23 @@ impl Display for UpdateStatus {
UpdateStatus::NoUpdate => write!(f, "No update"),
UpdateStatus::UpdateInstalled => write!(f, "Update installed"),
UpdateStatus::UpdateHadError => write!(f, "Update had error"),
UpdateStatus::UpdateIsBadPatch => write!(
f,
"Update available but previously failed to install. Not installing."
),
}
}
}
/// Returned when a call to `init` is not successful.
/// Returned when a call to `init` is not successful. These indicate that the specific call to
/// `init` was not successful, but the library may still be in a valid state (e.g., if
/// `AlreadyInitialized` is returned, the library is still initialized). Callers can safely ignore
/// these errors if they are not interested in the specific reason why `init` failed.
#[derive(Debug, PartialEq)]
pub enum InitError {
InvalidArgument(String, String),
AlreadyInitialized,
FailedToCleanUpFailedPatch,
}
impl std::error::Error for InitError {}
@@ -62,6 +70,9 @@ impl Display for InitError {
write!(f, "Invalid Argument: {name} -> {value}")
}
InitError::AlreadyInitialized => write!(f, "Shorebird has already been initialized."),
InitError::FailedToCleanUpFailedPatch => {
write!(f, "Failed to clean up after a failed patch.")
}
}
}
}
@@ -128,6 +139,34 @@ fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result<PathBuf
first.map(PathBuf::from)
}
pub fn with_state<F, R>(f: F) -> anyhow::Result<R>
where
F: FnOnce(&UpdaterState) -> anyhow::Result<R>,
{
with_config(|config| {
let state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
f(&state)
})
}
pub fn with_mut_state<F, R>(f: F) -> anyhow::Result<R>
where
F: FnOnce(&mut UpdaterState) -> anyhow::Result<R>,
{
with_config(|config| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
f(&mut state)
})
}
/// Initialize the updater library.
/// Takes a `AppConfig` struct and a yaml string.
/// The yaml string is the contents of the `shorebird.yaml` file.
@@ -161,31 +200,46 @@ pub fn init(
return Err(InitError::AlreadyInitialized);
}
let _ = with_config(|config| {
UpdaterState::load_or_new_on_error(
handle_prior_boot_failure_if_necessary()
}
/// If, at initialization time, we detect that we were in the process of booting a patch, report a
/// failure to boot for that patch and queue an event to report the failure.
pub fn handle_prior_boot_failure_if_necessary() -> Result<(), InitError> {
with_config(|config| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
)
.on_init()
});
);
if let Some(patch) = state.currently_booting_patch() {
state.record_boot_failure_for_patch(patch.number)?;
state.queue_event(PatchEvent::new(
config,
EventType::PatchInstallFailure,
patch.number,
))?;
}
Ok(())
Ok(())
})
.map_err(|e| {
error!("Failed to clean up after a failed patch: {:?}", e);
InitError::FailedToCleanUpFailedPatch
})
}
/// Whether the auto-update flag is set to true in the config.
pub fn should_auto_update() -> anyhow::Result<bool> {
with_config(|config| Ok(config.auto_update))
}
fn patch_check_request(config: &UpdateConfig, state: &UpdaterState) -> PatchCheckRequest {
let latest_patch_number = state.latest_seen_patch_number();
fn patch_check_request(config: &UpdateConfig) -> PatchCheckRequest {
// Send the request to the server.
PatchCheckRequest {
app_id: config.app_id.clone(),
channel: config.channel.clone(),
release_version: config.release_version.clone(),
patch_number: latest_patch_number,
platform: current_platform().to_string(),
arch: current_arch().to_string(),
}
@@ -193,17 +247,9 @@ fn patch_check_request(config: &UpdateConfig, state: &UpdaterState) -> PatchChec
fn check_for_update_internal() -> anyhow::Result<PatchCheckResponse> {
let (request, url, request_fn) = with_config(|config| {
// Load UpdaterState from disk
// If there is no state, make an empty state.
let state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
// Get the required info to make the request.
Ok((
patch_check_request(config, &state),
patch_check_request(config),
patches_check_url(&config.base_url),
config.network_hooks.patch_check_request_fn,
))
@@ -290,31 +336,17 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
// Saves state to disk (holds Config lock while writing).
let config = copy_update_config()?;
// We should never try to write this state as some other writer may be
// racing with us, we should get a new state inside a lock if we want
// to write.
let read_only_state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
// We discard any events if we have more than 3 queued to make sure
// we don't stall the client.
let events = read_only_state.copy_events(3);
let events = with_state(|state| Ok(state.copy_events(3)))?;
for event in events {
let result = crate::network::send_patch_event(event, &config);
if let Err(err) = result {
error!("Failed to report event: {:?}", err);
}
}
// We're abusing the config lock as a UpdateState lock for now.
let read_only_state = with_config(|_| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
let request = with_mut_state(|state| {
// This will clear any events which got queued between the time we
// loaded the state now, but that's OK for now.
let result = state.clear_events();
@@ -322,11 +354,10 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
error!("Failed to clear events: {:?}", err);
}
// Update our outer state with the new state.
Ok(state)
Ok(patch_check_request(&config))
})?;
// Check for update.
let request = patch_check_request(&config, &read_only_state);
let patch_check_request_fn = &(config.network_hooks.patch_check_request_fn);
let response = patch_check_request_fn(&patches_check_url(&config.base_url), request)?;
if !response.patch_available {
@@ -335,6 +366,25 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
let patch = response.patch.ok_or(UpdateError::BadServerResponse)?;
// Don't install a patch if it has previously failed to boot.
let is_known_bad_patch = with_state(|state| Ok(state.is_known_bad_patch(patch.number)))?;
if is_known_bad_patch {
info!(
"Patch {} has previously failed to boot, skipping.",
patch.number
);
return Ok(UpdateStatus::UpdateIsBadPatch);
}
// If we already have the latest available patch downloaded, we don't need to download it again.
let next_boot_patch = with_mut_state(|state| Ok(state.next_boot_patch()))?;
if let Some(next_boot_patch) = next_boot_patch {
if next_boot_patch.number == patch.number {
info!("Patch {} is already installed, skipping.", patch.number);
return Ok(UpdateStatus::NoUpdate);
}
}
let download_dir = PathBuf::from(&config.download_dir);
let download_path = download_dir.join(patch.number.to_string());
// Consider supporting allowing the system to download for us (e.g. iOS).
@@ -356,16 +406,11 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
// We're abusing the config lock as a UpdateState lock for now.
// This makes it so we never try to write to the UpdateState file from
// two threads at once. We could give UpdateState its own lock instead.
with_config(|_| {
with_mut_state(|state| {
let patch_info = PatchInfo {
path: output_path,
number: patch.number,
};
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
// Move/state update should be "atomic" (it isn't today).
state.install_patch(&patch_info, &patch.hash, patch.hash_signature.as_deref())?;
info!("Patch {} successfully installed.", patch.number);
@@ -434,28 +479,19 @@ where
/// 2. `start_update_thread()`
/// 3. `report_launch_failure()`
pub fn next_boot_patch() -> anyhow::Result<Option<PatchInfo>> {
with_config(|config| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
Ok(state.next_boot_patch())
})
with_mut_state(|state| Ok(state.next_boot_patch()))
}
/// The patch which is currently booted. This is `None` until
/// `report_launch_start()` is called at which point it is copied from
/// `next_boot_patch`.
/// The patch that was last successfully booted. If we're booting a patch for the first time, this
/// will be the previous patch (or None, if there was no previous patch) until the boot is
/// reported as successful.
///
/// TODO: This should always return the currently running patch, even if it has not been marked as
/// good or bad. Presently, users of the shorebird_code_push package will never get the wrong
/// patch number from this function because a launch will have been reported to be either a
/// success or a failure before they can call this function.
pub fn current_boot_patch() -> anyhow::Result<Option<PatchInfo>> {
with_config(|config| {
let state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
Ok(state.current_boot_patch())
})
with_state(|state| Ok(state.current_boot_patch()))
}
pub fn report_launch_start() -> anyhow::Result<()> {
@@ -465,19 +501,12 @@ pub fn report_launch_start() -> anyhow::Result<()> {
// next is now "patch to boot next"
info!("Reporting launch start.");
with_config(|config| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
let next_boot_patch = match state.next_boot_patch() {
Some(patch) => patch,
None => return Ok(()),
};
state.record_boot_start_for_patch(next_boot_patch.number)
with_mut_state(|state| {
if let Some(next_boot_patch) = state.next_boot_patch() {
state.record_boot_start_for_patch(next_boot_patch.number)
} else {
Ok(())
}
})
}
@@ -502,15 +531,7 @@ pub fn report_launch_failure() -> anyhow::Result<()> {
if mark_result.is_err() {
error!("Failed to mark patch as bad: {:?}", mark_result);
}
let event = PatchEvent {
app_id: config.app_id.clone(),
arch: current_arch().to_string(),
identifier: EventType::PatchInstallFailure,
patch_number: patch.number,
platform: current_platform().to_string(),
release_version: config.release_version.clone(),
timestamp: time::unix_timestamp(),
};
let event = PatchEvent::new(config, EventType::PatchInstallFailure, patch.number);
// Queue the failure event for later sending since right after this
// function returns the Flutter engine is likely to abort().
state.queue_event(event)
@@ -550,15 +571,11 @@ pub fn report_launch_success() -> anyhow::Result<()> {
let config_copy = config.clone();
std::thread::spawn(move || {
let event = PatchEvent {
app_id: config_copy.app_id.clone(),
arch: current_arch().to_string(),
patch_number: booting_patch.number,
platform: current_platform().to_string(),
release_version: config_copy.release_version.clone(),
identifier: EventType::PatchInstallSuccess,
timestamp: time::unix_timestamp(),
};
let event = PatchEvent::new(
&config_copy,
EventType::PatchInstallSuccess,
booting_patch.number,
);
let report_result = crate::network::send_patch_event(event, &config_copy);
if let Err(err) = report_result {
error!("Failed to report successful patch install: {:?}", err);
@@ -595,8 +612,9 @@ mod tests {
use crate::{
cache::{PatchInfo, UpdaterState},
config::{testing_reset_config, with_config},
events::EventType,
network::{testing_set_network_hooks, NetworkHooks, PatchCheckResponse},
time, ExternalFileProvider,
time, with_state, ExternalFileProvider, Patch,
};
#[derive(Debug, Clone)]
@@ -707,23 +725,74 @@ mod tests {
#[serial]
#[test]
fn ignore_version_after_marked_bad() {
fn ignore_version_after_marked_bad() -> anyhow::Result<()> {
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, None);
// Install a fake patch.
install_fake_patch(1).unwrap();
assert!(crate::next_boot_patch().unwrap().is_some());
install_fake_patch(1)?;
assert!(crate::next_boot_patch()?.is_some());
// pretend we booted from it
crate::report_launch_start().unwrap();
crate::report_launch_success().unwrap();
assert!(crate::next_boot_patch().unwrap().is_some());
crate::report_launch_start()?;
crate::report_launch_success()?;
assert!(crate::next_boot_patch()?.is_some());
with_state(|state| {
assert!(!state.is_known_bad_patch(1));
Ok(())
})?;
// boot again, this time failing
crate::report_launch_start().unwrap();
crate::report_launch_failure().unwrap();
crate::report_launch_start()?;
crate::report_launch_failure()?;
// Technically might need to "reload"
// ask for current patch (should get none).
assert!(crate::next_boot_patch().unwrap().is_none());
assert!(crate::next_boot_patch()?.is_none());
with_state(|state| {
assert!(state.is_known_bad_patch(1));
Ok(())
})?;
Ok(())
}
#[serial]
#[test]
fn reports_patch_install_failure_if_patch_was_booting() -> anyhow::Result<()> {
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, None);
install_fake_patch(1)?;
with_config(|config| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
assert_eq!(state.next_boot_patch().unwrap().number, 1);
Ok(())
})?;
// Pretend we started to boot from it, but don't report success or failure.
crate::report_launch_start()?;
with_state(|state| {
assert_eq!(state.currently_booting_patch().unwrap().number, 1);
// We should have no queued events
assert!(state.copy_events(1).is_empty());
Ok(())
})?;
// Pretend we're starting the app a second time
init_for_testing(&tmp_dir, None);
with_state(|state| {
assert!(state.currently_booting_patch().is_none());
// We should now have a queued PatchInstallFailure event
let events = state.copy_events(1);
assert_eq!(events.len(), 1);
assert_eq!(events[0].identifier, EventType::PatchInstallFailure);
Ok(())
})?;
Ok(())
}
#[test]
@@ -866,6 +935,84 @@ mod tests {
.unwrap();
}
#[serial]
#[test]
fn does_not_download_known_bad_patch() -> anyhow::Result<()> {
let mut server = mockito::Server::new();
let check_response = PatchCheckResponse {
patch_available: true,
patch: Some(crate::network::Patch {
number: 1,
download_url: "download_url".to_string(),
hash: "hash".to_string(),
hash_signature: None,
}),
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
.mock("POST", "/api/v1/patches/check")
.with_status(200)
.with_body(check_response_body)
.create();
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, Some(&server.url()));
let mut updater_state = with_config(|config| {
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
state.record_boot_failure_for_patch(1)?;
Ok(state)
})?;
// Make sure we're starting with no next boot patch.
assert!(updater_state.next_boot_patch().is_none());
let result = super::update()?;
// Ensure that we've skipped the known bad patch.
assert_eq!(result, crate::UpdateStatus::UpdateIsBadPatch);
assert!(updater_state.next_boot_patch().is_none());
Ok(())
}
#[serial]
#[test]
fn does_not_download_already_installed_patch() -> anyhow::Result<()> {
let patch_number = 1;
let mut server = mockito::Server::new();
let check_response = PatchCheckResponse {
patch_available: true,
patch: Some(Patch {
number: patch_number,
hash: "#".to_string(),
download_url: "download_url".to_string(),
hash_signature: None,
}),
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
.mock("POST", "/api/v1/patches/check")
.with_status(200)
.with_body(check_response_body)
.create();
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, Some(&server.url()));
install_fake_patch(patch_number)?;
let update_status = super::update()?;
assert_eq!(update_status, crate::UpdateStatus::NoUpdate);
Ok(())
}
#[serial]
#[test]
fn events_sent_during_update() {