Skip to content
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

Display full response body on error #55

Merged
merged 1 commit into from
Jul 7, 2021

Conversation

orangejulius
Copy link
Member

@orangejulius orangejulius commented Jun 30, 2021

The compare app has always had fairly cryptic messaging when there was an error response from one of the Pelias servers.

It ignores any actual response content from Pelias, and instead displays a static error message ('failed to load json') as well as a bit of synthesized JSON. Neither of these contain much information at all.

Before
Screen Shot 2021-06-30 at 11 43 10 AM

This PR changes around the behavior so that in the case of any error-like response (really, a response that does not have the geocoding section that Pelias generally returns), the actual response body is displayed, which might help debugging.

After
Screen Shot 2021-06-30 at 11 47 01 AM

The compare app has always had fairly cryptic messaging when there was
an error response from one of the Pelias servers.

It ignores any actual response content from Pelias, and instead displays
an error message ('failed to load json') as well as a bit of static,
synthesized JSON. Neither of these contain much information at all.

This PR changes around the behavior so that in the case of any
error-like response (really, a response that does not have the
`geocoding` section that Pelias generally returns), the actual response
body is displayed, which might help debugging.
Copy link
Member

@missinglink missinglink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me 👍

Copy link
Member

@Joxit Joxit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😉

@orangejulius orangejulius merged commit 00c88a4 into master Jul 7, 2021
@orangejulius orangejulius deleted the display-response-body-on-error branch July 7, 2021 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants