Unable To Detect An Error Has Occurred Within GraphQL Request - imdbGraphQLPHP Exits With An Untrappable Error

[mosource21]

Problem If there is a problem with the GraphQL request (IMDB down, IMDB busy, DNS problem, Timeout etc) execution continues and many PHP warnings are output and eventually an PHP error is output (as the data imdbGraphQLPHP is expecting from the GraphQL request is not available) but you have no means of detecting that an error has occurred

Solution Throw an exception in the GraphQL.php doRequest method so any calling code can trap this exception and act accordingly. This is similar to what tboothman originally did but now simplified for your imdbGraphQLPHP implementation.

Change the following in GraphQL.php at line 100

Remove return new \StdClass();

Replace With throw new \Exception("Failed to retrieve query [$queryName]");

[duck7000]

Mm actually the logger is called in that case but yes logger has to be enabled in config to be able to see the error

Now that i think about this i never understood why you would need 2 different types of error reporting, but in this case when logger is disabled there is no way to see any errors.

If i implant your solution it will but the ordinary user will see a exception error but doesn't know/understand what is going on?

And logger is in my view misleading as it does not actually "log" anything to a log file, it outputs an error to the browser.

So i'm not sure what the best solution is?

[duck7000]

After some reading how throw exceptions work i think there is indeed a flaw in doRequest. If there is any error or things go wrong the script continues with a blank object, this can work but is obviously not the best solution as there is no point continuing. Best would be to break execution, and indeed throw exception is the best to do so.

So i think that re adding the exception classes: require_once DIR . '/src/Imdb/Exception.php'; require_once DIR . '/src/Imdb/Exception/Http.php';

And adding your solution (or what tboothman did?) Would a config option be necessary? or always true? I think that is not the right way as a user can disable this, keeping the same problem.

[mosource21]

As you mentioned in a previous issue you are trying to keep your library as simple as possible so just replacing line 100 with my solution is all that is needed - it should just use the built in PHP Exception class (I think). With your aim of keeping things simple I do not think there is any need of config options or the additional exception classes. Perhaps I am too focused on what works for me though.

If you want to easily mimic a problem with the GraphQL request temporarily use the following. It sets the timeout to 1 millisecond which makes the request always fail but needs a newish PHP/CURL version to support the _MS option. curl_setopt($this->ch, CURLOPT_TIMEOUT_MS, 1);

Depending on what data you are requesting you will get several PHP warnings and/or errors being output due to no GraphQL data being returned.

[duck7000]

Thanks for your explanation

Okay i'll implant your solution as it is indeed a simple fix.

thanks for checking my code, I'll appreciate it!

[duck7000]

fixed in latest commit

[mosource21]

Yes both commits looking good. Timeout and throwing error now working great. Many thanks for including the changes.

[duck7000]

thanks for testing!