fixing error message in a log statement#13791
Conversation
TZubiri
requested review from
AnuGayan,
Arshardh,
chamilaadhi and
tharindu1st
as code owners
There was a problem hiding this comment.
Pull Request Overview
Fix incorrect exception message in ResourceAdminServiceClient.getMimeTypeFromHuman by updating the thrown message text.
- Update exception message string to better reflect the operation.
- No functional changes beyond message update.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| 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); | ||
|
|
||
| } |
There was a problem hiding this comment.
Catching Exception and rethrowing a new generic Exception obscures the original, more specific exception type and adds little value. Prefer either: (a) remove the try/catch and let the stub's exception propagate, or (b) rethrow the original exception (throw e;) to preserve type, or (c) wrap in a more specific checked exception used by this client rather than java.lang.Exception.
| return resourceAdminServiceStub.getMimeTypeFromHuman(mediaType); | ||
| } catch (Exception e) { | ||
| String msg = "get human readable media type error "; | ||
| String msg = "get mime type from human error "; |
There was a problem hiding this comment.
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).
| String msg = "get mime type from human error "; | |
| String msg = "Error getting MIME type from human-readable media type for: " + mediaType; |
There was a problem hiding this comment.
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.
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
It seems that the old message had been incorrectly copy pasted without modifying the error message.
Another issue might be that the potentially specific Exception is cast to the base Exception type, losing the more specific error signal.
What's interesting to note is that the try catch statement adds no value, removing it altogether would have actually fixed both issues.