Add additional error information

[lpatino10]

An issue was submitted a while back where users using a certain API couldn't retrieve a correlation_id field in their error response due to the way we were handling and wrapping errors. This field was particularly important for support people trying to resolve errors.

This PR changes the main exception class ServiceResponseException to handle parsing for error messages, which used to be done in WatsonService it also changes what is provided for the user, removing the raw Response object in favor of message (the parsed error message) and debuggingInfo, which is a Map of the rest of the response.

A test has been added to validate the debugging information.

[germanattanasio]

I quickly looked at the PR and have 2 questions

• Can you get the response body even if it doesn't match the classic error or error_message or message json response?
• Can you get ALL the response headers?

[lpatino10]

Good questions. I just made some changes so here are the answers:

1. If we can't parse the error message by looking for error, error_message, or message, we'll supply a default error message. These are the same as before.
2. I just added a getHeaders() method to the ServiceResponseException class to get an instance of our custom Headers class. All other body info will be in debuggingInfo.

[padamstx]

by looking for error, error_message, or message

IIRC, this is the set of properties that needs to be commonized across all the languages, per https://github.ibm.com/arf/planning-sdk-squad/issues/818

[lpatino10]

Yep, those are the ones @padamstx. I'll send a message out about those now and we can hold off on merging this if you want until we get that coordinated.

[germanattanasio]

@lpatino10 some services like Discovery return an errors array with Error objects. Is this taking that case into account?

[padamstx]

I think we can go ahead and merge this PR now. We just need to remember to update the other core packages to use the same common set of response property names.

[padamstx]

Looks good. Just one comment to think about.

[padamstx]

It might be cleaner if the various MESSAGE_ERROR* constants were just a String[] where the elements are an ordered list of the property names that we'll look for. Then the various if/elseif/.../else blocks would become a loop over the elements in the array and we just bail out after finding the first match. This way, when (and I mean when) the list of properties changes, it will be easy to change them :)

[lpatino10]

That does sound cleaner. I'll make that change along with (maybe) the one to handle the new error spec.

[lpatino10]

@germanattanasio are you talking about this spec?: https://pages.github.ibm.com/CloudEngineering/api_handbook/design/errors.html

Taj just sent me that and I'm not sure if we have other SDKs specifically handling it. We can add that for the major release though.

[germanattanasio]

looks good

[padamstx]

LGTM

[mkistler]

Regarding:

some services like Discovery return an errors array with Error objects.

I have complained about this design, which is actually the recommended approach in the API Handbook, since I first saw it. I think this form of error response is wrong-headed. The rationale is that there could be multiple errors and we should report all of them on the response. But then how do we convert this into a meaningful error that software can process?

cc: @jsstylos