API (js) - handle non-2** (and 1**) responses as errors #783
Conversation
himdel
changed the title
| if (e.message) { | ||
| $log.log(e.message); | ||
| console.error(e.message); | ||
| } |
There was a problem hiding this comment.
I don't see a reason to remove $log here.
How about using $log.error() instead of console.error (will also take care of the CC error)
There was a problem hiding this comment.
Looks like $log doesn't seem to work together with out toast-errors-in-devel-mode code :(.
And since we're not using $log anywhere else, I see no reason to fix that... do you?
EDIT: also, the point of CC warning about console.* is not to never use console.*, but to only use it in cases where console output is desired, as opposed to leaving debugging code in, or pretending to notify users by doing console.*. So... using $log instead of console just to prevent CC errors would be exactly the wrong solution. (And yes, we should add a CC warning about $log as well.)
app/assets/javascripts/miq_api.js
Outdated
| @@ -158,6 +158,9 @@ | |||
| if (response.status === 204) // No content | |||
| return null; | |||
|
|
|||
| if (response.status >= 300) // Not 1** or 2** | |||
| return Promise.reject(response.json()); | |||
|
|
|||
There was a problem hiding this comment.
Curly braces for if please...
when API returns a 404 and you have code like.. ``` API.get(...) .then(handleSuccess) .catch(handleError) ``` You'd expect `handleError` to be called. But, until now, these would all call `handleSuccess`. Fixing that. We'll try .json() on error anyway, since error messages from API are JSON. If the response is not json, it will throw anyway.
so that we at least see the error in devel mode and can add a proper error handler handleFailure should really flash too, but.. that would be an incompatible change so leaving for a separate PR.
|
Checked commits https://github.com/himdel/manageiq-ui-classic/compare/fa897106f631e2167e7e3912f086a177a3be84f3~...bc8a622d652086bca2944ea90b8133f26e53bb9f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
When API returns a 404 and you have code like..
You'd expect
handleErrorto be called.But, until now, these would all call
handleSuccess.Fixing that :).
Also, updating
miqService.handleFailuretoconsole.errorthe error, so that we can see it in devel mode at least.(A separate PR that makes
handleFailureactually handle the failure and show a flash message will follow, that will need either a separate function or some more changes to be compatible with existing code. - so, after PTO :).)Cc: @ZitaNemeckova , @mzazrivec