Follow up from #456: Update path qualifier and introduce module-specific Result

[shinghim]

From a comment in #456:

If I were forced to come up with one TODO on this PR, or feedback for next time, I would suggest that crate::db::Error were not imported in this file, and rather the function signature was qualified as db::Error since db::Error is not semantically the top level Error for lib.rs. We don't have one, which is why there's no conflict, but HandlerError would be the closest thing.

Writing this also made me notice that some Result types are duplicated in a bunch of places, so there is an argument to be made for

db::Result = Result<Vec, Error>
HandlerResult = Result<Response<BoxBody<Bytes, hyper::Error>>, HandlerError>
The result types are really getting nitty gritty but I figure it can't hurt to share

Updating to db::Error made sense, and I had to change db::Result = Result<Vec<u8>, Error> to Result<T> = anyhow::Result<T, Error> since pub async fn new(timeout: Duration, db_host: String) -> RedisResult<Self> didn't return a Vec<u8>

I wasn't sure about the HandlerResult update. In lib.rs, there are other functions such as listen_tcp and listen_tcp_with_tls that don't return a Result that can be converted into Result<Response<BoxBody<Bytes, hyper::Error>>, HandlerError>, so I didn't add the HandlerResult bit

[coveralls]

Pull Request Test Coverage Report for Build 12802427071

Details

• 9 of 11 (81.82%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.05%) to 60.713%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin-directory/src/db.rs	7	8	87.5%
payjoin-directory/src/lib.rs	2	3	66.67%

Totals
Change from base Build 12795849419:	-0.05%
Covered Lines:	2930
Relevant Lines:	4826

---

💛 - Coveralls

[DanGould]

This is what I requested and a quick turnaround

I had to change db::Result = Result<Vec, Error> to Result = anyhow::Result<T, Error> since pub async fn new(timeout: Duration, db_host: String) -> RedisResult didn't return a Vec

I don't see the change to anyhow::Result, am I missing something? Can you point it out to me?

@nothingmuch can you take a look at the second commit here. Although it is what I asked for I'm not sure that my request was appropriate. Maybe I'm just bummed that this adds an extra line.

[shinghim]

@DanGould whoops, i meant to say i switched it to Result<T> = core::result::Result<T, Error> in db.rs instead of Result = Result<Vec, Error>

[nothingmuch]

@nothingmuch can you take a look at the second commit here. Although it is what I asked for I'm not sure that my request was appropriate. Maybe I'm just bummed that this adds an extra line.

IIUC, then I agree, for generic type names like Error I find fully qualified references more readable than importing the base name. I'm slightly confused though, did I miss something about the 2nd commit in particular?

[DanGould]

@nothingmuch The 2nd commit change makes use of fully qualified errors, it creates a Result alias

[nothingmuch]

@nothingmuch The 2nd commit change makes use of fully qualified errors, it creates a Result alias

the use of db::Result in lib.rs makes sense and is seems consistent with and a refinement of the 1st commit, so 👍 as far as i'm concerned