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

refactor: Use Result to indicate failure in mark_patch_bad (#66)

Browse Source 
I'm not sure if this is better or worse, but it does make testing
a bit more clear.
This commit is contained in:
Eric Seidel
2023-08-04 09:55:35 -07:00
committed by GitHub
parent 15ab0f2295
commit 1841f59b94
2 changed files with 25 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
library/src/cache.rs
+19 -23
View File
@@ -9,7 +9,7 @@ use std::fs::File;
use std::io::{BufReader, BufWriter};
use std::path::{Path, PathBuf};
use anyhow::Context;
use anyhow::{bail, Context, Result};
use serde::{Deserialize, Serialize};
use crate::updater::UpdateError;
@@ -88,34 +88,30 @@ impl UpdaterState {
self.failed_patches.iter().any(|v| v == &patch_number)
}
// TODO(eseidel): Should return Result instead of logging, the c_api
// layer can log if desired.
pub fn mark_patch_as_bad(&mut self, patch_number: usize) {
pub fn mark_patch_as_bad(&mut self, patch_number: usize) -> Result<()> {
if self.is_known_good_patch(patch_number) {
warn!("Tried to report failed launch for a known good patch. Ignoring.");
return;
bail!("Tried to report failed launch for a known good patch. Ignoring.");
}
if self.is_known_bad_patch(patch_number) {
return;
if !self.is_known_bad_patch(patch_number) {
// This is at least info! since we're in a failure state and want to log.
info!("Marking patch {} as bad", patch_number);
self.failed_patches.push(patch_number);
}
// This is at least info! since we're in a failure state and want to log.
info!("Marking patch {} as bad", patch_number);
self.failed_patches.push(patch_number);
Ok(())
}
// TODO(eseidel): Should return Result instead of logging, the c_api
// layer can log if desired.
pub fn mark_patch_as_good(&mut self, patch_number: usize) {
pub fn mark_patch_as_good(&mut self, patch_number: usize) -> Result<()> {
if self.is_known_bad_patch(patch_number) {
warn!("Tried to report successful launch for a known bad patch. Ignoring.");
return;
bail!("Tried to report successful launch for a known bad patch. Ignoring.");
}
if self.is_known_good_patch(patch_number) {
return;
if !self.is_known_good_patch(patch_number) {
self.successful_patches.push(patch_number);
}
self.successful_patches.push(patch_number);
Ok(())
}
fn load(cache_dir: &Path) -> anyhow::Result<Self> {
@@ -475,12 +471,12 @@ mod tests {
let tmp_dir = TempDir::new("example").unwrap();
let mut state = test_state(&tmp_dir);
let bad_patch = fake_patch(&tmp_dir, 1);
state.mark_patch_as_bad(bad_patch.number);
state.mark_patch_as_bad(bad_patch.number).unwrap();
let number = bad_patch.number;
assert!(state.install_patch(bad_patch).is_err());
// Calling a second time should not error.
state.mark_patch_as_bad(number);
state.mark_patch_as_bad(number).unwrap();
}
#[test]
@@ -488,8 +484,8 @@ mod tests {
let tmp_dir = TempDir::new("example").unwrap();
let mut state = test_state(&tmp_dir);
let bad_patch = fake_patch(&tmp_dir, 1);
state.mark_patch_as_bad(bad_patch.number);
state.mark_patch_as_good(bad_patch.number);
assert!(state.mark_patch_as_bad(bad_patch.number).is_ok());
assert!(state.mark_patch_as_good(bad_patch.number).is_err());
assert!(state.is_known_bad_patch(bad_patch.number));
assert!(!state.is_known_good_patch(bad_patch.number));
}
@@ -499,11 +495,11 @@ mod tests {
let tmp_dir = TempDir::new("example").unwrap();
let mut state = test_state(&tmp_dir);
let patch = fake_patch(&tmp_dir, 1);
state.mark_patch_as_good(patch.number);
state.mark_patch_as_good(patch.number).unwrap();
assert!(state.is_known_good_patch(patch.number));
assert!(!state.is_known_bad_patch(patch.number));
// Marking it twice doesn't change anything.
state.mark_patch_as_good(patch.number);
state.mark_patch_as_good(patch.number).unwrap();
assert!(state.is_known_good_patch(patch.number));
assert!(!state.is_known_bad_patch(patch.number));
}
library/src/updater.rs
+6 -2
View File
@@ -364,7 +364,9 @@ pub fn report_launch_failure() -> anyhow::Result<()> {
.ok_or(anyhow::Error::from(UpdateError::InvalidState(
"No current patch".to_string(),
)))?;
state.mark_patch_as_bad(patch.number);
// Ignore the error here, we'll try to activate the next best patch
// even if we fail to mark this one as bad (because it was already bad).
let _ = state.mark_patch_as_bad(patch.number);
state
.activate_latest_bootable_patch()
.map_err(|err| anyhow::Error::from(err))
@@ -382,7 +384,9 @@ pub fn report_launch_success() -> anyhow::Result<()> {
.ok_or(anyhow::Error::from(UpdateError::InvalidState(
"No current patch".to_string(),
)))?;
state.mark_patch_as_good(patch.number);
// Ignore the error here, we'll try to activate the next best patch
// even if we fail to mark this one as good.
let _ = state.mark_patch_as_good(patch.number);
state
.save()
.map_err(|_| anyhow::Error::from(UpdateError::FailedToSaveState))