Use a consistent pattern for error handling about quilkin HOT 7 CLOSED

I can create a PR to replace the tokio::io::Result usages and link that to close this PR 👍

from quilkin.

Given that we're also planning on allowing people to use this as a library - it seems like we should have our own Error enum(s), and expose those as public, rather than expose the underlying libraries to the outside world (especially if we ever want those to change).

(To be honest, I only recently started to look more at error handling, and getting more of a handle on it)

I'd be inclined to handcraft our of own enums/structs to start, and if we get more complicated, bring in a more sophisticated library as needed. WDYT?

from quilkin.

+1 on writing our own structs, it makes sense to introduce a library if/when we know it adds significant value.

from quilkin.

I feel like we've resolved this for now - what do you think, can we close this?

from quilkin.

We currently have some places where we needed to return proper errors, e.g in server.rs we still pass tokio result - the idea was to resolve those cases in this ticket.

from quilkin.

We currently have some places where we needed to return proper errors, e.g in server.rs we still pass tokio result - the idea was to resolve those cases in this ticket.

Ah yes! I had forgotten! Excellent point.

from quilkin.

Just did a quick review of server.rs and session.rs -- I think we are good to close this now.

One thing I did note - we probably should create a ticket to review of public methods, and restrict them to only being available to the crate or smaller.

Because we do still expose tokio::io::Result from some public methods, which is fine if they are only used internally (and restricted as such), but not if they are used as a public library api surface.

How does that sound?

from quilkin.