cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

RestClient no longer returns an HTTPError #118

Closed nedredmond closed 1 year ago

nedredmond commented 1 year ago

Before the bump to v2, I could do this:

import {
  ...
  "github.com/cli/go-gh/v2/pkg/api"
}
...

err = restClient.Get(path, &resp)
if err != nil {
  if err.(api.HTTPError).StatusCode == 404 {
    log.Fatal("special message")
  }
  log.Fatal("general message")
}

Since updating, that is no longer a valid casting. I tried doing errors.As, but that didn't seem to work either.

Based on some other repositories I found, I tried this:

import {
  ...
  ghApi "github.com/cli/go-gh/v2/pkg/api"
  "github.com/cli/cli/v2/api"
}
...

restClient, err := ghApi.DefaultRESTClient()
if err != nil {
  log.Fatal(err)
}
...

err = restClient.Get(path, &resp)
if err != nil {
  var httpErr api.HTTPError
  if errors.As(err, &httpErr) && httpErr.StatusCode == 404 {
    log.Fatal("special message")
  }
  log.Fatal("general message")
}

This is always going to the general Fatal response message and not the special message. It seems that the errors.As casting is not working here, and the error is a simple string error. I'd love to be able to parse this status code without examining the error string (which does return "HTTP 404: Not Found...").

Is there some new, undocumented method to consume these errors, or was this an accidental regression? Thanks.

samcoe commented 1 year ago

@nedredmond This is not a regression but I did a poor job of explaining this change in the the release notes. I wrote

Change methods on HTTPError and GraphQLError custom error types to take pointers as method receivers.

but didn't explain when that would make an impact. This is the exact case, when trying to do type assertions on the errors returned from the clients. You should be able to change your code to assert that the type returned is *api.HTTPError rather than api.HTTPError.

  if err.(*api.HTTPError).StatusCode == 404 {
    log.Fatal("special message")
  }

Hope this helps. Going to close this for now but let me know if this does not fix your issue.

nedredmond commented 1 year ago

Thanks @samcoe-- I saw the note and thought it was a much more complicated migration somehow. Appreciate the help!