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

Fix error propagating logic #515

Merged
merged 3 commits into from
Jan 29, 2024

Conversation

ellnix
Copy link
Collaborator

@ellnix ellnix commented Jan 17, 2024

Pull Request

Related issue

Fixes #513

What does this PR do?

  • Meilisearch::ApiError parses a server's JSON response into its
    @http_body instance variable which stores a hash.
  • When version_error_handler propagates ApiError, it passes the @http_body
    hash as http body which causes ApiError to attempt to parse it
    again as JSON, throwing a TypeError.
  • This PR makes the http_body parameter polymorphic, allowing either a hash or a JSON string to be passed.

Meilisearch::ApiError parses a server's JSON response into its
`@http_body` instance variable which stores a hash.
When `version_error_handler` propagates ApiError, it passes `@http_body`
as the server http body which causes ApiError to attempt to parse it
again as JSON, throwing a TypeError.
Allows easier propagation of ApiError.
@ellnix ellnix added the bug Something isn't working label Jan 17, 2024
Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (40f80cb) 99.62% compared to head (bbcfa7f) 99.81%.
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #515      +/-   ##
==========================================
+ Coverage   99.62%   99.81%   +0.18%     
==========================================
  Files           9        9              
  Lines         540      542       +2     
==========================================
+ Hits          538      541       +3     
+ Misses          2        1       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ellnix ellnix requested a review from curquiza January 17, 2024 11:19
@curquiza curquiza requested review from brunoocasali and removed request for curquiza January 17, 2024 13:06
@curquiza
Copy link
Member

@ellnix I put @brunoocasali as reviewer: he's the maintainer of this repo. I just try to help him sometimes when he has too much on his plate 😊
For this PR, I would rather him to review

@ellnix
Copy link
Collaborator Author

ellnix commented Jan 17, 2024

@ellnix I put @brunoocasali as reviewer: he's the maintainer of this repo. I just try to help him sometimes when he has too much on his plate 😊 For this PR, I would rather him to review

Oh okay then, I apologize for requesting a review. 👍

@curquiza
Copy link
Member

Oh okay then, I apologize for requesting a review. 👍

Don't worry, you are right, if you hesitate, you can also ping me 😊

Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

LGTM!!

@ellnix
Copy link
Collaborator Author

ellnix commented Jan 29, 2024

bors merge

Copy link
Contributor

meili-bors bot commented Jan 29, 2024

@meili-bors meili-bors bot merged commit 5e2d354 into meilisearch:main Jan 29, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError is raised instead of Meilisearch::ApiError on invalid filter to documents api
3 participants