Display error when service instance creation fails #1173
Conversation
|
Due to the structure of the form component's html, server errors are displayed before the 'There were errors submitting the form'. |
|
|
||
| serviceActions.errorCreateInstance(originalError); | ||
|
|
||
| const actionInfo = spy.getCall(0).args[0]; |
There was a problem hiding this comment.
I'm not using the test helpers here as I was having a hard time getting the test to validate when the arguments passed to the dispatch function are objects.
| const argumentError = { response: { data: { error_code: 'CF-MessageParseError' } } }; | ||
| const spaceError = { response: { data: { error_code: 'CF-ServiceInstanceInvalid' } } }; | ||
| const serverErrorMsg = | ||
| 'Error #500: please contact cloud.gov support for help troubleshooting this issue.'; |
There was a problem hiding this comment.
let me know if you think this would be something for another PR: would be nice to remove the cloud.gov hard coding so that in the future we can achieve #790 eventually
There was a problem hiding this comment.
Yes I agree. Do we have any concept of the app environment baked into the build process or anything? I think we can have webpack set a global variable that will get injected into the app code. Then 'cloud.gov' could just be whatever platform the dashboard is running on.
In that case, the client setting up the dashboard would need to replace 'cloud.gov' with whatever the name of their platform is.
| errorCreateInstance(err) { | ||
| errorCreateInstance(error) { | ||
| const { response } = error; | ||
| const safeError = (response && response.data) || { code: 500 }; |
There was a problem hiding this comment.
coming from a non-JS perspective:
it looks like (response && response.data) could evaluate to true or false.
if true, safeError would be set to true.
if false, it would set safeError to {code: 500};
Now your tests, tell me it doesn't work that way, but just could be a little confusing at first glance.
There was a problem hiding this comment.
also, don't you also neederror_code to make sure this is a safe error?
There was a problem hiding this comment.
Using this construct in JS, if the first argument evaluates to true, and the second argument is also true, the second argument will be executed. In this instance, executing means storing the value of the second argument in a variable, but you could, for example, pass a function as the second argument.
If the second argument is also false, the fallback will be executed instead.
I concede that this might look a little confusing but it is a pretty common in JS.
If I misunderstood the question, and you already knew this, sorry 😬 !
There was a problem hiding this comment.
@jcscottiii For your second comment, error_code needn't be present, the store checks for the existence of one and returns the generic 500 error message if it isn't present.
There was a problem hiding this comment.
Regarding the first one, I've seen it done like this with ternary operators commonly. condition ? happy case : sad case but sounds good.
There was a problem hiding this comment.
Regarding the second one: ahh i gotcha.
el-mapache
force-pushed
the
ab-service-instance-bind-errors
branch
from
3bbe8f9 to
b53d3b5
Compare
|
I mistakenly misnamed this PR's branch, it covers error handling when creating a service instance, not binding a service instance. |
Screenshots!