Closed Bug 401000 Opened 13 years ago Closed 13 years ago

make helper method calGetString() exception safe

Categories

(Calendar :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ssitter, Assigned: ssitter)

Details

Attachments

(1 file)

Follow up for Bug 400997: An error in the German translation caused an exception in calGetString(). To avoid issues like this we should consider making the helper methods exception safe. A solution might be to catch the exception, report the error and return e.g. a default error string.
In general I like to throw exceptions for errors and wouldn't change that. It's more important that client code is exception safe, e.g. assures staying in a consistent state in case of an exception.
However in the case of calGetString, it hinders a good/quick diagnose that something is broken (because calGetString() is used to display an error box). IMO we should catch the exception, dump it into the error console, and return a string that states the exception (and maybe the arguments of calGetString).
Assignee: nobody → ssitter
Status: NEW → ASSIGNED
Attachment #290576 - Flags: review?(daniel.boelzle)
Comment on attachment 290576 [details] [diff] [review]
make calGetString() exception safe, rev0

>+        Components.utils.reportError(s + "\n" + ex);
Since js console doesn't mind line feeds, I've replaced that part with " Error: ".

r=dbo, thanks for the patch!

Checked in on HEAD and MOZILLA_1_8_BRANCH.
Attachment #290576 - Flags: review?(daniel.boelzle) → review+
(In reply to comment #1)
> In general I like to throw exceptions for errors and wouldn't change that.

Ok. In this case I would suggest that we resolve this bug as fixed and don't touch the other functions.
Target Milestone: --- → 0.8
Resolving as Fixed per comment #4.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Summary: make helper methods (e.g. calGetString) exception safe → make helper method calGetString() exception safe
You need to log in before you can comment on or make changes to this bug.