-
Notifications
You must be signed in to change notification settings - Fork 24
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
Centralize API request/response debug logging and response decoding #113
Comments
Upon a first look, it makes sense for the extra checks (and the debug logging) to be incorporated into As for how to handle those cases (return an error array or throw exceptions), I don't have an informed opinion. I trust your judgment on that one. |
In the past few days, I read up on (PHP) exception handling. While opinions vary (depending on the programmer's preference for them, even) about when to pass on error returns vs. when to throw an exception, one good practice appears to be to throw only for unexpected (run-time) errors. Error values then cover situations one can expect, e.g. invalid credentials at login or a duplicate file getting uploaded. In a chain of calls, e.g. in our case So this looks like a practical, pragmatic approach in our case, and that's what I implemented in #136. |
I agree, that approach sounds sensible. |
PR #112 introduced the
Wikimate::request()
method, invoked for all API calls. The latter still check the response body for some error situations -- the doc-page test and the decoding result -- and perform the JSON decode. The error checks happen only intoken()
andlogin()
anyway. These could be moved intorequest()
, but it is also worth considering not to return an error array but throw an exception. I have to ponder this a bit longer. Any thoughts, @waldyrious ?Secondly, the debug logging of the request array and decoded response array can also be centralized in
request()
.The text was updated successfully, but these errors were encountered: