-
Notifications
You must be signed in to change notification settings - Fork 909
fixing error message in a log statement #13791
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -221,7 +221,7 @@ public String getMimeTypeFromHuman(String mediaType) throws Exception { | |
| try { | ||
| return resourceAdminServiceStub.getMimeTypeFromHuman(mediaType); | ||
| } catch (Exception e) { | ||
| String msg = "get human readable media type error "; | ||
| String msg = "get mime type from human error "; | ||
| throw new Exception(msg, e); | ||
|
|
||
| } | ||
|
Comment on lines
221
to
227
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message is still unclear/grammatically awkward and ends with a trailing space. Consider a clearer, actionable message and include context, e.g., String msg = "Error getting MIME type from human-readable media type for: " + mediaType; (remove trailing space and improve capitalization).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't quite matter or is highly secondary.
In situations where only the error message is logged, and not the exception stack, the error message serves more as an error ID than a useful descriptor, the logging debugging workflow will be grepping the exact error message through the codebases to find the exact codepath.
Logging the actual exception stack as-is might be a reasonable suggestion (with the counterargument that it might pollute log files), but is somewhat beyond the scope of this bug, which is that the bug id/msg is a duplicate, and thus code execution information is lost on logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally the + mediaType introduces the possibility of type errors, which would risk an exception during exception error, making this suggestion hugely net negative.
We should think of code in a catch block as a second serve in tennis, slow and careful instead of featureful and optimistic