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

Gracefully handle HTTP 204 response bodies #1098

Merged
merged 7 commits into from
Mar 21, 2023
Merged

Gracefully handle HTTP 204 response bodies #1098

merged 7 commits into from
Mar 21, 2023

Conversation

ClaytonPassmore
Copy link
Contributor

@ClaytonPassmore ClaytonPassmore commented Jan 12, 2023

Description

Fixes #1097

What is the problem it is solving?

The HTTP client raises a NoMethodError when the server responds with a 204. I believe this is because HTTParty doesn't bother to parse the response body in certain situations, and just supplies nil as the response body instead.

What is the context of these changes?

It's not entirely clear what this question is asking. Please refer to #1097 for more context.

What is the impact of this PR?

It allows people to use the library to do things like delete discount codes and price rules without raising an exception when Shopify responds with a 204, the expected response.

How has this been tested?

I wrote a test first to confirm it would trigger a failure, which it did (see screenshot). The failure went away with my fix.

I have used the patched fork of the gem to delete discount codes and price rules in a development environment, which it does without raising an exception.

Screenshot 2023-01-12 at 11 12 47 AM

Checklist:

  • My commit message follow the pattern described in here
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the project documentation.
  • I have added a changelog line.

@ClaytonPassmore
Copy link
Contributor Author

travolta_shopify

Copy link

@frenkel frenkel 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, plain and simple!

Copy link
Contributor

@nelsonwittwer nelsonwittwer left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this! Thank you for providing the fix to this issue!

@ClaytonPassmore
Copy link
Contributor Author

@nelsonwittwer thank you for taking a look 🙌

@nelsonwittwer nelsonwittwer merged commit 4e74cca into Shopify:main Mar 21, 2023
@ClaytonPassmore ClaytonPassmore deleted the fix/http-204 branch March 21, 2023 18:32
nickmealey pushed a commit to heliumdevelopment/shopify-api-ruby that referenced this pull request Jun 6, 2023
* Gracefully handle HTTP 204 response bodies

* Add changelog entry for Shopify#1098

* Fix up changelog formatting

* Rubocop fixup
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.

REST client raises an exception on HTTP 204 response
3 participants