golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.73k stars 17.5k forks source link

proposal: must.Do #54297

Open bradfitz opened 2 years ago

bradfitz commented 2 years ago

I always find it tedious to use httputil.ReverseProxy because you usually need a *url.URL and you can't make one easily without a few lines of boilerplate.

Proposal: add url.MustParse like regexp.MustCompile, template.Must, etc.

(Alternatively: a generic must.Do somewhere in std)

cc @neild

mvdan commented 2 years ago

Alternatively: a generic must.Do somewhere in std

I'd support this: we already have other similar APIs like https://pkg.go.dev/text/template#Must and https://pkg.go.dev/regexp#MustCompile, and there's really nothing special about them. Some standard support for must could also be nice for one-off scripts or short examples, where panicking on errors is perfectly fine, e.g. https://go-review.googlesource.com/c/go/+/196497.

mvdan commented 2 years ago

I realise that some of what I wrote above overlaps with the try proposal, but I think they are distinct, because handling errors by panicking is generally not a good idea in most Go code. I think must would see most of its use in init funcs or global variables, one-off programs, examples, and other similar cases where returning a proper error isn't possible or necessary.

earthboundkid commented 2 years ago

I literally wrote a commit last night and thought, I should propose url.MustParse because I was wrapping so many can't fail package level URLs: https://github.com/spotlightpa/almanack/commit/40fcde98ffa3c9327800e8da68e8750780325d29

bradfitz commented 2 years ago

We're using this for our codebase: https://pkg.go.dev/tailscale.com/util/must#Get

// Get returns v as is. It panics if err is non-nil.
func Get[T any](v T, err error) T {
    if err != nil {
        panic(err)
    }
    return v
}

We've found it surprisingly useful (var target = must.Get(url.Parse(...)), etc)

earthboundkid commented 2 years ago

must.Get and must.Do are better names than try.To and try.Must. I will borrow your names. :-)

I'm not sure if this should be in the standard library or not. We've all written quickie scripts with die(error) or check(error) that panics or calls log.Fatal, so it's definitely a very common practice in real world code, but it's also probably good if there is some friction to doing it because it's not actually a best practice. Then again maybe the standard library could offer it and people could decide for themselves if they want to use it. +0

dsnet commented 2 years ago

We've all written quickie scripts with die(error) or check(error) that panics or calls log.Fatal

One question is intent. The tailscale must package is intended to be safe to use in production code where the naming of the package makes it very clear that dying is part of its behavior when errors occur. It's great for initializing global variables.

In sufficiently large Go "scripts", you may want some degree of error handling, where the do-or-die approach of must is no longer a good fit. The github.com/dsnet/try package is intended to be halfway between what must does and the typical approach of error handling in Go.

dottedmag commented 1 year ago

Some more prior art from the codebase I work on: https://pkg.go.dev/github.com/ridge/must/v2 - must.OK and must.Do

Eyal-Shalev commented 3 months ago

Some more prior art from the codebase I work on: https://pkg.go.dev/github.com/ridge/must/v2 - must.OK and must.Do

FYI https://pkg.go.dev/github.com/ridge/must/v2 is deprecated and the fork https://github.com/dottedmag/must is recommended