-
Notifications
You must be signed in to change notification settings - Fork 72
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
DisqusAPIError constructor extends PHP Exception but switches arguments around #8
Comments
Well, from a purely logical standpoint, it always makes sense to have the code first in an API response because it will always exist, whereas the message will not. Suffice it to say that the rest of the web has pretty much standardized an API response/error to look like (code, message) and so in this case it does makes sense. I guess more formally, an API response/error is different than an exception and thus should be treated differently. |
@astilla Do you happen to know what the $code is used for in PHP internally? The code in our APIError is definitely not intended to represent anything more than what our API tells you it means. |
As of now it actually doesn't work. When I got a "400 Bad Request" HTTP error, I have to change the code like this to have a meaningful error:
|
To elaborate on ceefour, the problem here is that $data->response (disqusapi.php, line 118) isn't a string (when the response code isn't 200), but an array containing an object. Php's Exception class expects $message to be a string, not an array or an object. The $data->code is fine I believe. |
I just noticed that DisqusAPIError subclasses a normal PHP Exception but for some weird reason decided to switch the arguments around - why?
DisqusAPIError: __construct($code, $message)
Standard PHP Exception: __construct ([ string $message = "" [, int $code = 0 [, Exception $previous = NULL ]]] )
Doesn't make sense to me to switch the params around - it just leads to confusion!
The text was updated successfully, but these errors were encountered: