pds api 127: improved error messaging #95
Conversation
Changed all of the AcceptTypeException to return 406 instead of 501. Improved the message for the exception to include all of the known Accept: content types allowed.
|
Ready for review. |
There was a problem hiding this comment.
Thanks @al-niessner , that works well. the most critical comment is how the message shows to the user.
...main/java/gov/nasa/pds/api/engineering/elasticsearch/business/RequestAndResponseContext.java
Outdated
Show resolved
Hide resolved
| throw new ApplicationTypeException("The given application type, " + String.valueOf(this.format) + ", is not known by RquestAndResponseContext."); | ||
| for (String key : this.formatters.keySet()) | ||
| { | ||
| log.warn(" key: " + String.valueOf(key)); |
There was a problem hiding this comment.
Do we need a warning for this message ? I would think of DEBUG, unless I misunderstood what it does.
There was a problem hiding this comment.
Need in what way? Will the program work without the warning, yes. When a user says they got a 406 on Saturday will you be able to find it in the log if not posted, no. So define what you mean by need.
Warnings are usually issued when the program detects something fishy (a handled exception) notifying the initiator of the fishy bit that something looks wrong but was handled albeit maybe not how the initiator wanted so they may want to try again -- think or google compiler warnings. Warnings differs from errors in that the program does not crash and burn. Debug on the other hand is way of tracing the program to determine where a bug is hiding or may exist or prove its existance.
For this message when a user mentions days later that a 406 was returned (something fishy but handled) we would have the appropriate level of logging.
If you want it to be debug then simply say so directly and I will change it. One could argue that given errors are being returned to the user the logging is superfluous.
...main/java/gov/nasa/pds/api/engineering/elasticsearch/business/RequestAndResponseContext.java
Outdated
Show resolved
Hide resolved
🗒️ Summary
Changed all of the AcceptTypeException to return 406 instead of 501.
Improved the message for the exception to include all of the known Accept: content types allowed.
Note, if a Accept: content type is not supported, then the result always comes out as JSON. If the request was 'Accept: text/htlm' then the chosen serializer is JSON.
⚙️ Test Data and/or Report
♻️ Related Issues
#446