(fix) O3-902: Improve error message clarity and consistency in error handling#1725
(fix) O3-902: Improve error message clarity and consistency in error handling#1725Safa-khanum wants to merge 1 commit intoopenmrs:mainfrom
Conversation
|
|
ibacher
left a comment
ibacher
left a comment
There was a problem hiding this comment.
First, please do not make up a ticket number. These references need to be valid and we do not use PR numbers as ticket numbers. Second, I don't really see the value in changing the messages here. Third, while it's a good idea to handle new cases, it's not at all clear why you switched from the else if structure that existed and prevented redundant comparisons. Fourth, I don't really see a lot of value in just pre-pending the string "Error:" to things.
| } else if (thing === null) { | ||
| return Error(`'null' was thrown as an error`); | ||
| } else if (typeof thing === 'object') { | ||
| } | ||
|
|
||
| if (thing === null) { | ||
| return new Error(`Received null instead of a valid error object.`); | ||
| } | ||
|
|
||
| if (thing === undefined) { | ||
| return new Error(`Received undefined instead of a valid error object.`); | ||
| } |
There was a problem hiding this comment.
Why did you change this from an else if? Also, these two branches feel like they could be combined.
| } | ||
| } | ||
|
|
||
| // string / number / boolean |
There was a problem hiding this comment.
It's better to avoid comments of ovious code
3 participants
Requirements
If applicable
Summary
This PR improves error message clarity and consistency in the global error handling module.
Previously, non-Error inputs produced inconsistent and less descriptive messages. This change standardizes error formatting and provides clearer context for debugging.
Changes
Why this matters
Clear and consistent error messages improve debugging and developer experience, especially when handling unexpected inputs.
Screenshots
N/A
Related Issue
Inspired by #902
Other
Non-breaking internal improvement.