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

proposal: must: Do #54297

Open
bradfitz opened this issue Aug 5, 2022 · 11 comments
Open

proposal: must: Do #54297

bradfitz opened this issue Aug 5, 2022 · 11 comments
Labels
Milestone

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Aug 5, 2022

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

@gopherbot gopherbot added this to the Proposal milestone Aug 5, 2022
@mvdan
Copy link
Member

mvdan commented Aug 6, 2022

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
Copy link
Member

mvdan commented Aug 6, 2022

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
Copy link
Contributor

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: spotlightpa/almanack@40fcde9

@rsc rsc moved this to Incoming in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@bradfitz
Copy link
Contributor Author

bradfitz commented Aug 10, 2022

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
Copy link
Contributor

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
Copy link
Member

dsnet commented Aug 10, 2022

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
Copy link
Contributor

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
Copy link

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

@ashurbekovz
Copy link

I have thoughts on why sticking must.Do in the standard library might not be a good idea:

  • by implication Must is not always if err != nil { panic(err) }. Theoretically it could be if err != nil && err != io.EOF { panic(err) } or something similar.
  • A package-specific Must can change the number of accepted arguments when the major version of the package changes. This may make it easier for package users to migrate to a new version.

If we introduce generic Must, the following problems will become relevant:

  • coders may forget the package-specific Must where it is needed.
  • Introducing package-specific Must will in a sense break package backward compatibility, since users will have to replace generic Must everywhere to avoid unexpected behavior.

This seems to me to be too big a price to pay for saving a few lines of code.

@dottedmag
Copy link
Contributor

dottedmag commented Mar 8, 2025

  • by implication Must is not always if err != nil { panic(err) }. Theoretically it could be if err != nil && err != io.EOF { panic(err) } or something similar.

Only if you define "Must" as "panics on any programming error, and only on programming errors". Don't do that. Error values are used to propagate all kinds of non-errors, so "Must" ought to be a shortcut, not a semantically new idea.

This seems to me to be too big a price to pay for saving a few lines of code.

The bigger impact is ability to use err-emitting code in an expression. It quite often allows one to convert a rambling 10-20 line procedure to 2-3 liner.

@ashurbekovz
Copy link

Only if you define "Must" as "panics on any programming error, and only on programming errors". Don't do that. Error values are used to propagate all kinds of non-errors, so "Must" ought to be a shortcut, not a semantically new idea.

Ok, I agree that in general defining “Must” as “panics on any programming error, and only on programming errors” is a bad idea. I implicitly assumed something like that when I wrote the backwards compatibility argument, so the argument doesn't work.

The bigger impact is ability to use err-emitting code in an expression. It quite often allows one to convert a rambling 10-20 line procedure to 2-3 liner.

I meant a few lines of code to write your package in the repository. I don't deny that in the context of a particular repository the use of such a package (self-written or third-party) may be justified.

@seankhliao seankhliao changed the title proposal: must.Do proposal: must: Do Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

8 participants