Skip to content

Commit de30640

Browse files
committed
Replace unwraps with descriptive error messages.
1 parent a4af1cb commit de30640

File tree

2 files changed

+29
-15
lines changed

2 files changed

+29
-15
lines changed

src/app.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ async fn download_and_cache_s3_object<'a>(
257257
panic!("Invalid cache key: {}", key);
258258
}
259259

260-
if let Some(metadata) = chunk_cache.get_metadata(&key).await {
260+
if let Some(metadata) = chunk_cache.get_metadata(&key).await? {
261261
if !allow_cache_auth_bypass {
262262
// To avoid having to include the S3 client ID as part of the cache key
263263
// (which means we'd have a separate cache for each authorised user and

src/chunk_cache.rs

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,13 @@ impl ChunkCache {
117117
/// # Arguments
118118
///
119119
/// * `key`: Unique key identifying the chunk
120-
pub async fn get_metadata(&self, key: &str) -> Option<Metadata> {
121-
self.cache.get_metadata(key).await
120+
pub async fn get_metadata(&self, key: &str) -> Result<Option<Metadata>, ActiveStorageError> {
121+
match self.cache.get_metadata(key).await {
122+
Ok(value) => Ok(value),
123+
Err(e) => Err(ActiveStorageError::ChunkCacheError {
124+
error: format!("{:?}", e),
125+
}),
126+
}
122127
}
123128

124129
/// Retrieves chunk `Bytes` from the cache for a unique key.
@@ -261,7 +266,13 @@ impl SimpleDiskCache {
261266
async fn load_state(&self) -> State {
262267
let file = self.dir.join(&self.name).join(SimpleDiskCache::STATE_FILE);
263268
if file.exists() {
264-
serde_json::from_str(fs::read_to_string(file).await.unwrap().as_str()).unwrap()
269+
serde_json::from_str(
270+
fs::read_to_string(file)
271+
.await
272+
.expect("Failed to read cache state file")
273+
.as_str(),
274+
)
275+
.expect("Failed to deserialise cache state")
265276
} else {
266277
State::new(self.prune_interval_seconds)
267278
}
@@ -276,7 +287,7 @@ impl SimpleDiskCache {
276287
let file = self.dir.join(&self.name).join(SimpleDiskCache::STATE_FILE);
277288
fs::write(file, serde_json::to_string(&data).unwrap())
278289
.await
279-
.unwrap();
290+
.expect("Failed to write cache state file");
280291
}
281292

282293
/// Converts a chunk key into a string that can be used for a filename.
@@ -326,16 +337,19 @@ impl SimpleDiskCache {
326337
/// # Arguments
327338
///
328339
/// * `key`: Unique key identifying the chunk
329-
async fn get_metadata(&self, key: &str) -> Option<Metadata> {
340+
async fn get_metadata(&self, key: &str) -> Result<Option<Metadata>, String> {
330341
match fs::read_to_string(
331342
self.dir
332343
.join(&self.name)
333344
.join(self.filename_for_key(key).await + ".meta"),
334345
)
335346
.await
336347
{
337-
Ok(content) => Some(serde_json::from_str(content.as_str()).unwrap()),
338-
_ => None,
348+
Ok(content) => Ok(Some(serde_json::from_str(content.as_str()).unwrap())),
349+
Err(err) => match err.kind() {
350+
std::io::ErrorKind::NotFound => Ok(None),
351+
_ => Err(format!("{}", err)),
352+
},
339353
}
340354
}
341355

@@ -388,11 +402,11 @@ impl SimpleDiskCache {
388402
// Remove the chunk file
389403
fs::remove_file(path.join(self.filename_for_key(key).await))
390404
.await
391-
.unwrap();
405+
.expect("Failed to remove chunk file");
392406
// Remove the metadata file
393407
fs::remove_file(path.join(self.filename_for_key(key).await + ".meta"))
394408
.await
395-
.unwrap();
409+
.expect("Failed to remove chunk metadata file");
396410
// Update the global state
397411
state.current_size_bytes -= data.size_bytes;
398412
self.save_state(state).await;
@@ -522,11 +536,11 @@ mod tests {
522536
assert_eq!(metadata.get(key_1).unwrap().size_bytes, value_1.len());
523537
assert_eq!(cache_item_1.unwrap(), Some(value_1));
524538
assert_eq!(
525-
cache.get_metadata(key_1).await.unwrap().expires,
539+
cache.get_metadata(key_1).await.unwrap().unwrap().expires,
526540
metadata.get(key_1).unwrap().expires
527541
);
528542
assert_eq!(
529-
cache.get_metadata(key_1).await.unwrap().size_bytes,
543+
cache.get_metadata(key_1).await.unwrap().unwrap().size_bytes,
530544
metadata.get(key_1).unwrap().size_bytes
531545
);
532546

@@ -542,11 +556,11 @@ mod tests {
542556
assert_eq!(metadata.get(key_2).unwrap().size_bytes, value_2.len());
543557
assert_eq!(cache_item_2.unwrap(), Some(value_2));
544558
assert_eq!(
545-
cache.get_metadata(key_2).await.unwrap().expires,
559+
cache.get_metadata(key_2).await.unwrap().unwrap().expires,
546560
metadata.get(key_2).unwrap().expires
547561
);
548562
assert_eq!(
549-
cache.get_metadata(key_2).await.unwrap().size_bytes,
563+
cache.get_metadata(key_2).await.unwrap().unwrap().size_bytes,
550564
metadata.get(key_2).unwrap().size_bytes
551565
);
552566

@@ -560,7 +574,7 @@ mod tests {
560574
assert!(!metadata.contains_key(key_1));
561575
assert!(metadata.contains_key(key_2));
562576
assert_eq!(cache_item_1.unwrap(), None);
563-
assert!(cache.get_metadata(key_1).await.is_none());
577+
assert!(cache.get_metadata(key_1).await.unwrap().is_none());
564578
}
565579

566580
#[tokio::test]

0 commit comments

Comments
 (0)