You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Carrying over @deet's comment from #9 to a new bug:
I've run into some other issues with using this function and those that return its
result, and I'm not sure how they should be addressed. (Let me know if you want
this as a separate issue.) Specifically, clients can't determine whether the call
failed due to network error or because of something application-level (like non-2XX
response). So far it seems necessary to do a substring match for "HTTP response
is not 200/OK as expected" to determine that the call failed due to a server
response. Any suggestions?
Although I'm not entirely sure the best way to handle this canonically in Go, here's what I'm thinking:
http://blog.golang.org/error-handling-and-go has a section called "The error type". That section stars by discussing creating errors with errors.New and fmt.Errorf, which is basically what this library is doing now.
Starting with the "In many cases fmt.Errorf is good enough, but..." section, it goes on to describe defining an error struct, and then allowing callers to examine the details of that error using type assertion.
So here are two ideas along those lines:
Define a single new error type "OauthError" that extends the error type, and we can insert any additional information in there. I'm not 100% sure what would go in the struct, but you could imagine something like an httpStatusCode field, that was empty if no HTTP request was made.
Define multiple new error types OauthHttpError, OauthNetworkError, GenericOauthError, whatever. Then the user can do type assertion and figure out which kind of error they got back.
My first reaction is that the first option involves less type assertion, so seems a little cleaner, but both seem like they'd be totally fine.
If you're want to, feel free to send a patch. Otherwise I'll try to write some code for this maybe in the next week or two.
The text was updated successfully, but these errors were encountered:
Carrying over @deet's comment from #9 to a new bug:
Although I'm not entirely sure the best way to handle this canonically in Go, here's what I'm thinking:
http://blog.golang.org/error-handling-and-go has a section called "The error type". That section stars by discussing creating errors with errors.New and fmt.Errorf, which is basically what this library is doing now.
Starting with the "In many cases fmt.Errorf is good enough, but..." section, it goes on to describe defining an error struct, and then allowing callers to examine the details of that error using type assertion.
So here are two ideas along those lines:
My first reaction is that the first option involves less type assertion, so seems a little cleaner, but both seem like they'd be totally fine.
If you're want to, feel free to send a patch. Otherwise I'll try to write some code for this maybe in the next week or two.
The text was updated successfully, but these errors were encountered: