Shopify / Shopify/shopify-api-php

Wrap JsonException for better differentiation

Open
#443 0 comments 0 reactions 0 assignees View on GitHub

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:

  1. Build a new class named ResponseJsonException extending the original JsonException. Add an additional field for the plain text response body.
  2. Wrap the following line with try/catch block
    https://github.com/Shopify/shopify-api-php/blob/f4d177e8ce062aa302aef10e29bc274ac26ccd19/src/Clients/HttpResponse.php#L48
  3. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.