|
|
|
|
|
|
by sethammons
544 days ago
|
|
|
no no no; do not return os.NotExists in both cases. The function needs to handle os.NotExists and then return mypkg.ErrOwnerNotExists or mypkg.ErrMissingConfig (or whatever names) depending on the state in the function. The os.NotExists error is an implementation detail that is not important to callers. Callers shouldn't care about files on disk as that is leaking abstraction info. What if the function decides to move those configs to s3? Then callers have to update to handle s3 errors? No way. Return errors specific to your function that abstract the underlying implementation. Edit: here is some sample code
https://go.dev/play/p/vFnx_v8NBDf Second edit: same code, but leveraging my other comment's kverr package to propagate context like kv pairs up the stack for logging:
https://go.dev/play/p/pSk3s0Roysm |
|
|
Distilling their implementation to the basics, that's what we get: typed errors that wrap the Go standard library's ones with custom logic. Frankly I doubt that the API your library exposes (kv maps) vs OPs typed structs, is better. Maybe their main issue is relying on stuffing all error types in the same module, instead of having each independent app coming up with their own, but probably that's because they need the behaviour for handling those errors at the top of the calling stack is uniform and has only one implementation.
A quick back of the napkin list for what an error needs to contain to be useful in a post execution debugging context would be:
* calling stack
* traceability info like (request id, trace id, etc)
* data for the handling code to make meaningful distinction about how to handle the error
I think your library could be used for the last two, but I don't know how you store calling stack in kv pairs without some serious handwaving. Also kv is unreliable because it's not compile time checked to match at both ends.