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

Adding error inspection of resource fetched by schema.php #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JorgenSmith
Copy link

Fixes up my previous PR #36 . In some cases the $config variable will hold a HTTP exception, and we need to catch this so as to enable more meaningful errors to be returned.

The patch will give me e.g.
Error fetching 'productVariant'. CHttpException: Unable to resolve the request "productVariant/options".

Instead of
`exception 'CException' with message 'Property "Restyii\Client\Schema\Resource.code" is not defined.``

@@ -115,6 +115,11 @@ protected function fetchResource($name, $refresh = false)
return false;
}

// detect http errors in resource fetch call
if(is_array($config) && array_key_exists('type', $config) && $config['type'] === 'CHttpException') {
Copy link
Member

Choose a reason for hiding this comment

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

we won't expose the exception type in future, or we cannot guarantee it will always be a CHttpException, let's just assume that if there's an error and if the response is an array, we should throw a CHttpException locally.

Copy link
Contributor

Choose a reason for hiding this comment

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

And to repeat myself: The exception we throw here, is not the same that we received from the API. We have a 502 in that case (bad gateway or uplink connection.).

Copy link
Author

Choose a reason for hiding this comment

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

@phpnode , Good point - however I'm not sure what the possible ['type']s would be. There could be - should we check if it contains the string "Exception" and then throw the 502?

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.

3 participants