Skip to content
This repository was archived by the owner on Apr 13, 2025. It is now read-only.

Handle non-JSON responses#118

Merged
forest merged 2 commits into
irphilli:masterfrom
shanecav84:non_json
Oct 31, 2017
Merged

Handle non-JSON responses#118
forest merged 2 commits into
irphilli:masterfrom
shanecav84:non_json

Conversation

@shanecav84

@shanecav84 shanecav84 commented Oct 31, 2017

Copy link
Copy Markdown
Contributor

The TrackerApi::Error in #61 is due to a Faraday::Error::ParsingError. The default error message for an overloaded server for Pivotal Tracker's app server, Phusion Passenger, is an HTML response with a 503 status code. TrackerApi::Client was expecting all responses to be JSON, and so passed each response to the JSON parser of Faraday::ResponseMiddleware, which raised the ParsingError. This is fixed by only passing JSON-related content-types to the middleware parser. Faraday will handle other responses normally; e.g. raising a ServerError for overloaded server responses. Closes #117.

In the course of sorting that out, I found that responses to attachment uploads also return text/html, which would again raise a ParsingError. This response is intentional as noted in the commit body for
6038ba8. This is fixed by setting our request's Accept header to application/json.

Responses to attachment upload requests return a JSON body with the Content-Type header set to 'text/html'. If we sepcify that we accept 'application/json', then the server will return the proper Content-Type header. This required a re-run of VCRs.

From the docs:

Note that the Content-type of the server response can lie. This behavior, while not strictly correct according to the protocol, is intended. There is a bug in Microsoft Internet Explorer that will cause it to navigate away from a page executing a JavaScript client app at the end of an upload if the server returns the (correct) type of application/json. The server will return the correct Content-type, instead of text/html, if the client includes application/json in the request's Accept header.

Source: https://www.pivotaltracker.com/help/api/#Uploading_File_Attachments
@coveralls

coveralls commented Oct 31, 2017

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 96.972% when pulling 6038ba8 on patientdev:non_json into 5bf3e1d on dashofcode:master.

@forest

forest commented Oct 31, 2017

Copy link
Copy Markdown
Contributor

+1

@forest forest merged commit 56fbf99 into irphilli:master Oct 31, 2017
@shanecav84

Copy link
Copy Markdown
Contributor Author

@forest Could you cut a release for this?

@forest

forest commented Jun 18, 2018

Copy link
Copy Markdown
Contributor

@shanecav84 done.

@shanecav84

Copy link
Copy Markdown
Contributor Author

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A TrackerApi::Error occurred in background at 2016-01-20 08:47

3 participants