Typed error for Not Found (404) responses

[joanlopez]

From the library's consumer point of view, the error for all HTTP responses where status code is >= 400 look the same. However, certain errors, like Not Found (404) require specific error handling (for instance, from Grizzly), and with the current approach that would require parsing the error message (bad idea!).

So, here I suggest to create a typed error for this specific case. We can add more typed errors later, if needed.

I don't think exposing this will hurt, but if there's any concern, I'd be happy to expose it as a behavioral error.

type errNotFound interface {
        NotFound() bool
}
 
// IsErrNotFound returns true if err is not found.
func IsErrNotFound(err error) bool {
        nf, ok := err.(errNotFound)
        return ok && nf.NotFound()
}

Thanks!

[inkel]

I love the idea! And I believe it will definitely be very useful. I'm approving because it LGTM to me as it is, but I've left some suggestions that would make it even better IMNSHO.

[inkel]

This could be simplified:

Suggested change

	return fmt.Sprintf("status: %d, body: %v", e.StatusCode, string(e.BodyContents))
	return fmt.Sprintf("status: %d, body: %s", e.StatusCode, e.BodyContents)

Which when combined with my other comment, would end up being:

Suggested change

	return fmt.Sprintf("status: %d, body: %v", e.StatusCode, string(e.BodyContents))
	return fmt.Sprintf("status: 404 Not Found, body: %s", e.BodyContents)

I honestly would even remove the status: 404 prefix, however, that would break backwards compatibility with consumers of this package that are looking for the prefix (like here).

[joanlopez]

Thanks @inkel, I applied your feedback at 110dc04, because it's indeed a nice idea!

However, I think there was an extra "Not Found" in your last suggestion, cause the former error was just printing the status code (integer), so I've just added the 404.

Could you confirm it looks good now, please? Thanks! 🙏🏻

[inkel]

Nice, thanks! Yeah, the extra not found was there on purpose, so in case the user prints the error message it shows indeed that it's not found (status: 404 says the same, but you require some technical knowledge); but I'm ok without having it in the message for now. Re-approving and merging!