Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.0 Roadmap - consolidate built-in wrappers and implement API and DX improvements. #40

Open
Southclaws opened this issue Sep 22, 2023 · 3 comments

Comments

@Southclaws
Copy link
Owner

Southclaws commented Sep 22, 2023

It's been about a year that this library has been in production use in multiple real-world revenue-generating products and I've got a good handle on the improvements I want to make and the direction this library is taking.

So, to mark this, I'd like to bundle the existing wrappers into a single package for a few reasons:

I'd like to use a single nice short package name for all the built-in wrappers, I'm thinking of just fault/f or fault/w. Mostly just for aesthetics, similar to how Zap and other log aggregators work:

Settled on wrap for context, tags and user-friendly error message reference wrappers

return fault.Wrap(err,
        wrap.Ctx(ctx),
        wrap.Tag(fault.TagNotFound),
        wrap.Msg("There was a technical error while retrieving your account"),
)

Previous ideas:

return fault.Wrap(err,
        f.WithCtx(ctx), // decorate the error with key-value metadata from context
        f.WithTag(ftag.NotFound), // categorise the error as a "not found"
        f.WithMsg("There was a technical error while retrieving your account"), // provide an end-user message
)

Or with:

return fault.Wrap(err,
        with.Ctx(ctx), // decorate the error with key-value metadata from context
        with.Tag(ftag.NotFound), // categorise the error as a "not found"
        with.Msg("There was a technical error while retrieving your account"), // provide an end-user message
)

Or just w

toqueteos pointed out this would be a bad idea as single letter symbols are often used for variable and parameter names.

return fault.Wrap(err,
        w.Ctx(ctx), // decorate the error with key-value metadata from context
        w.Tag(ftag.NotFound), // categorise the error as a "not found"
        w.Msg("There was a technical error while retrieving your account"), // provide an end-user message
)

Any ideas for this are welcome though! cc @matdurand @the-goodies @semihbkgr @toqueteos (just tagging folks who have contributed in case you have ideas or reservations around this!)

@Southclaws Southclaws changed the title v1.0: Finalise built-in wrappers in a single sub package ready for release v1.0 Roadmap - consolidate built-in wrappers and implement API and DX improvements. Sep 22, 2023
@toqueteos
Copy link
Contributor

toqueteos commented Sep 22, 2023

Hi! Thanks for the ping

I'd advise against single letter imports as those are usually taken by variables (n for counters, f for *os.File handles, w for io.Writers, i for loops, ...).

Why not just go all-in and get rid of the subpackage entirely? Just create some functional options that may or not be passed into fault.Wrap(err, ...).

Example:

return fault.Wrap(err,
        fault.WithCtx(ctx), // decorate the error with key-value metadata from context
        fault.WithTag(fault.TagNotFound), // categorise the error as a "not found"
        fault.WithMsg("There was a technical error while retrieving your account"), // provide an end-user message
)

fault is almost there already, it's a matter of just moving some code up to the root.

This would only increase the API surface from 7 to 10+ symbols + the leftover types from those subpackages that are needed (like ftag.Kind and the consts over there).

@Southclaws
Copy link
Owner Author

One of the main reasons I wanted to keep the wrappers separate was to emphasise that Fault itself is standalone and the real value it provides is the func (error) error interface - on which you can build your own wrappers specific to your organisation or product. That's the fundamental sell of Fault as a library, then the wrappers like fctx/ftag are effectively reference implementations or examples of how to implement them as such.

I agree with the single letter point though, that's something I didn't consider!

Brevity has its pros and cons, too short and the meaning is lost, too long and it becomes arduous to use and read.

Perhaps wrap is a logical package name for these:

return fault.Wrap(err,
        wrap.Ctx(ctx),
        wrap.Tag(fault.TagNotFound),
        wrap.Msg("There was a technical error while retrieving your account"),
)

@matdurand
Copy link
Contributor

Just my 2 cents, but if we're talking about 1.0 then I would consider the following. fmsg and ftag are packages used to store metadata about errors. There is obviously other types of metadata, for example, in my project, I basically copied ftag to make an ErrorCode concept that my http error handler is using to produce a response.

It seems to me that we could provide more generic version of ftag and fmsg, called for example fmeta to enable to storage of any metadata types on top of errors. Then we could rewrite ftag and even fmsg using this generic fmeta package. Obviously we would probably need to involve golang Generics at this point.

I feel it would provide far greater possibility for customizing fault for specific use cases.

If you think this is something worth pursuing, I could give it a try and come up with a first draft an a possible API and we could iterate on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants