x/tools/gopls: add "if err != nil" snippet completion for net/http's HandlerFunc

[marwan-at-work]

The net/http HandlerFunc is a very common function that Go developers write:

func handler(w http.ResponseWriter, r *http.Request)

Since the function doesn't return any errors, most users will have to handle the error in the following way:

func handler(w http.ResponseWriter, r *http.Request) {
  results, err := doSomething()
  if err != nil {
    http.Error(w, err.Error(), http.StatusInternalServerError)
    return
  }
  // use results
}

This can be quite tedious to write and people (aka me) often forget to add the "return" statement which ends up causing the net/http library to spit out errors about multiple status codes being written to the same response.

In production applications, Go developers typically structure their http servers in a way that will minimize the amount they have to do the HTTP error handling above. But they most likely still have to do it once per handler, or sometimes they want to prototype a server quickly without having to think about how to nicely structure their code to DRY up the error handling logic.

Therefore, as a continuation of improvements to our snippet completions mentioned in 43310, I'd love to add a completion that will simply offer to write the above error handling snippet for you. It should also put placeholders on the error and status code arguments while picking sane defaults such as err.Error() and http.StatusInternalServerError.

Since the code structure for snippet completions is already in place, adding this use case should be a minimal change. I've included a CL to show the type of change needed, which can be reviewed/merged if this issue is accepted.

Thanks!

[gopherbot]

Change https://golang.org/cl/351030 mentions this issue: internal/lsp: add snippet completion for errors inside http.HandlerFunc

[stamblerre]

Thanks for filing this and for mailing a CL! I've never really used net/http, and I'm wondering if there is some documentation about this being the canonical or industry-standard way of handling errors. It would be nice to have that linked in the code, especially in case people disagree.

[stamblerre]

/cc @golang/pkgsite team, since they have more experience with net/http

[jba]

That snippet is the sort of code you often see, but it is clumsy and error-prone (forgetting the return is bad). In pkgsite we define a wrapper so handlers can return errors. I imagine this is common too.

I don't think there's one canonical approach to handling errors. I also don't know what your criteria are for adding a snippet. The code given is certainly correct but will having the snippet discourage people from writing a wrapper, which is a cleaner approach?

[stamblerre]

Thanks for insight, @jba! We have been fairly hesitant about adding snippets, since it might seem like we're prescribing a "correct" or "Go team approved" way of doing things.

@marwan-at-work: How would you feel about an if err != nil { <> } snippet, where the user's cursor would be placed at the <>?

[marwan-at-work]

We have been fairly hesitant about adding snippets, since it might seem like we're prescribing a "correct" or "Go team approved" way of doing things.

That's totally fair :)

How would you feel about an if err != nil { <> } snippet, where the user's cursor would be placed at the <>?

I believe this already exists for editors like VSCode on the client side (if you type iferr and press enter). Another potential snippet is the following:

if err != nil { <> \n return }

If that looks good, I'd be happy to amend the CL.

But to be honest, I think the best solution here is to make gopls extensible so that people can write their own snippets that is specific to their usecases. Is there an issue open to follow or discuss this alternative?

@stamblerre thank you!

[stamblerre]

If that looks good, I'd be happy to amend the CL.

Yeah, that sounds good to me! We've been discussing moving the client side snippets to gopls anyway, and I like this because we'll still be offering that snippet, but just in the right contexts.

But to be honest, I think the best solution here is to make gopls extensible so that people can write their own snippets that is specific to their usecases. Is there an issue open to follow or discuss this alternative?

Does something like https://code.visualstudio.com/docs/editor/userdefinedsnippets#_create-your-own-snippets work for you? I know it is client-side, but I still think that's a little easier than integrating custom snippets with gopls.