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.

Sign up for GitHub

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

Jump to bottom

[feature] handle 504 response #503

Merged
merged 1 commit into from
Nov 12, 2019
Merged

[feature] handle 504 response #503

merged 1 commit into from
Nov 12, 2019

Conversation

adsteel
Copy link
Contributor

@adsteel adsteel commented Nov 11, 2019 •
edited
Loading

Addresses #502

Default handling for unexpected response bodies (here)
has revealed an unhandled 504 response from intercom.

Example message from UnexpectedResponseError:

Intercom::UnexpectedResponseError, Expected a JSON response body.
Instead got '<html> <head><title>504 Gateway Time-out</title></head>
<body bgcolor="white"> <center><h1>504 Gateway Time-out</h1></center>
</body> </html> ' with status code '504'.

jonnyom
jonnyom reviewed Nov 11, 2019
View reviewed changes
lib/intercom/version.rb Outdated
@@ -1,3 +1,3 @@
module Intercom #:nodoc:
VERSION = "3.9.3"
VERSION = "3.10.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind if we leave this change and then bump in a separate PR?

Modifying this version here affects the automatic bump command that we use when bundling releases 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course!

jonnyom
jonnyom reviewed Nov 11, 2019
View reviewed changes
Copy link
Contributor

@jonnyom jonnyom left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this again @adsteel this is great. One thing re the version but other than that it looks great.

@adsteel
[feature] handle 504 response
e450f85 
Default handling for unexpected response bodies ([here](#492))
has revealed an unhandled 504 response from intercom.

Example message from UnexpectedResponseError:

```
Intercom::UnexpectedResponseError, Expected a JSON response body.
Instead got '<html> <head><title>504 Gateway Time-out</title></head>
<body bgcolor="white"> <center><h1>504 Gateway Time-out</h1></center>
</body> </html> ' with status code '504'.
```
@adsteel
Copy link
Contributor Author

adsteel commented Nov 11, 2019

@jonnyom Removed the version bump. Ready for re-review.

jonnyom
jonnyom approved these changes Nov 12, 2019
View reviewed changes
Copy link
Contributor

@jonnyom jonnyom left a comment

Choose a reason for hiding this comment

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

Looks great to me, thank you @adsteel

I'll try to get a release out today.

@jonnyom jonnyom merged commit 7c2ca38 into intercom:master Nov 12, 2019
@jonnyom jonnyom mentioned this pull request Nov 12, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonnyom jonnyom jonnyom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adsteel @jonnyom