Closed emersion closed 2 years ago
Hm, "internal: return a DAVError instead of an Error" is a bit strange because it wraps DAVError
into HTTPError
, duplicating the HTTP code. We might want to pick between two strategies:
HTTPError
with an Error
wrapped in it to describe an XML DAV error. Remove DAVError
.DAVError
to describe an XML DAV error, and use HTTPError
for HTTP errors without an XML description. Never use Error
as a Go error value.Sorry, I never really looked much at the client code before, and it took me a moment to understand the error handling you added there :slightly_smiling_face:
I am not entirely sure which approach sounds better. On the one hand, since both are essentially different protocols, separating them seems reasonable. Yet, since they map so closely to each other, maybe not. I think in the client library, both would be easy to implement, but I have no idea how tedious proper handling would be in a client application.
On the server side, however, I'd be worried that separating them would result in a lot of noisy code, like "if it's a http error and code X or a dav error and code X, do this, same for code Y, etc." without adding much value. If the only difference is a human-readable error message, getting that from an optional wrapped error seems much easier. Hence I'd slightly lean toward your first suggestion. But it's not a strong opinion and I might very well be missing something.
Here's how the first approach would look like.
Looks good to me... :slightly_smiling_face:
Thanks for the feedback!
cc @bitfehler