Page MenuHomePhabricator

[backup] Introduce backup error
ClosedPublic

Authored by michal on Aug 25 2023, 9:49 AM.
Tags
None
Referenced Files
F3368574: D8952.id30327.diff
Mon, Nov 25, 8:17 PM
Unknown Object (File)
Fri, Nov 22, 2:46 PM
Unknown Object (File)
Fri, Nov 22, 9:24 AM
Unknown Object (File)
Mon, Nov 11, 11:34 AM
Unknown Object (File)
Fri, Nov 8, 6:43 PM
Unknown Object (File)
Fri, Nov 8, 10:53 AM
Unknown Object (File)
Sun, Oct 27, 5:01 PM
Unknown Object (File)
Sep 27 2024, 11:58 PM
Subscribers

Details

Summary

ENG-4380

Catch-all error for all errors that will be able to come up during handling of backup requests. For now it only has errors for blob client, but it will be expanded in the future diffs.

Test Plan

Create a service:

.service(web::resource("/hello").route(web::get().to(
       |blob: web::Data<BlobServiceClient>| async move {
         let _ = blob.get("asdasd").await.map_err(BackupError::from)?;
         Ok::<&str, BackupError>("world")
       },
     )))

Test a few different scenarios: blob error not available (not running), blob not existing

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek added inline comments.
services/backup/src/error.rs
28 ↗(On Diff #30327)

Btw - have you tested {err} vs {err:?} ? - Sometimes Debug is more meaningful and sometimes Display - I've never found a single good way.

This revision is now accepted and ready to land.Aug 25 2023, 1:04 PM
services/backup/src/error.rs
28 ↗(On Diff #30327)

Just tested it and it seems Display is fine. In this case (our own errors) I think we should try and keep the Display the preferred way (this is also why I prefer thiserror over derive_more because it requires you to be more explicit when it comes to the messages)