Shopify / Shopify/shopify-api-php
Wrap JsonException for better differentiation
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 473
- Forks
- 193
- Avg merge
- 5h 8m
- Merged PRs (30d)
- 1
Description
Hello
In our logs we see several times JsonExceptions (Syntax error) while sending data via the REST Api. In some cases we messed up the request data and in some cases the response from the REST API is containing invalid data.
For us this is causing difficulties for us as one public method (e.g. Fulfillment::save()) is able to throw in 2 different places JsonException. For us it is impossible to act here with code.
For example: We would like to build an automated retry feature if the response body had an invalid response. If the request body conatains the syntax error we do not need to retry and would like to to notify our developers instead to fix the issue.
So here our suggestion to quickly enable everyone to determine if it was a response triggered JsonException while keeping it backward compatible:
- Build a new class named ResponseJsonException extending the original JsonException. Add an additional field for the plain text response body.
- Wrap the following line with try/catch block
https://github.com/Shopify/shopify-api-php/blob/f4d177e8ce062aa302aef10e29bc274ac26ccd19/src/Clients/HttpResponse.php#L48 - In the catch block: simply throw the new ResponseJsonException with same message as the caught JsonException, add the plain boday and add the caught JsonException as previous
This will make sure all existing projects stay working and everyone is able to determine if it was an Json error in the response. You can do this as well with the Request body if you like.
If you need more details, please don't hesitate to ask.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at src/Clients/HttpResponse.php around line 48, where the response JSON is decoded. Review the existing JsonException handling and the proposed ResponseJsonException shape. Done means response-body JSON failures are distinguishable, retain the plain response body and previous exception, and preserve existing behavior for callers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- api
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100