-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add request and responses to debugging output #253
Conversation
body := io.TeeReader(res.Body, &bodyCopy) | ||
|
||
if err := json.NewDecoder(body).Decode(v); err != nil { | ||
debug.Println("res.Body:", bodyCopy.String()) | ||
return nil, fmt.Errorf("error parsing API response - %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should change this message to not return the error from the JSON decoder and add that error to the debug output. This message could be much friendlier and direct the user to either re-run the command with debug output or to file a ticket. If we get a request-id on the response back, we should display that as part of the friendly message. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
I'm going to look at the API part right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so it looks like this is when we do api.Submissions()
after the fetch, the API is returning an error as JSON:
# http://exercism.io/api/v1/exercises?key=
{
error: "Sorry—we can't figure out who you are. Double-check your API key in your exercism.io account page."
}
I'll change the API to return a zero-value for this call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh. This is kind of awful: the usual response from the API is in this format:
{
"go": [
{
"slug": "hello-world",
"state": "archived"
}
],
"ruby": [
{
"slug": "hello-world",
"state": "archived"
}
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, deploying API with reasonable empty state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetching without an api key now just fetches the exercise. 👍
Getting an error response from the server, now looks like this:
> ./cli fetch go leapa
verbose logging enabled
NewRequest: GET http://x.exercism.io/v2/exercises/go/leapa
StatusCode: 404
res.Body: {"error":"We don't have problem 'leapa' in 'go'"}
2015/11/21 11:16:59 unable to fetch problems (HTTP: 404) - We don't have problem 'leapa' in 'go'
We are displaying the error provided by the json response. Were any other changes required?
I'm wondering if we should always show the body response in verbose mode regardless of whether we have an error or not? It does add a lot of noise, but could be useful as we make changes to the erecism.io api. It could also be useful if, for example, someone has a proxy server that is mangling the response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How was the verbose logging activated in the above command? It doesn't look like a flag was passed to ./cli
.
I think you're right that we should always show the body in verbose mode, that would be immensely helpful. Maybe with a newline after it so that the "usual" output is separated a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I over-condensed the above. This is the real command:
go build ./exercism/main.go && ./main --verbose -c config.json fetch go leapa
Cool. I'll space out the responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good! I was worried I was missing something. Thanks.
af61bee
to
82ad98d
Compare
Add request and responses to debugging output
Helps with debugging situations like this:
exercism/go#202