Closed Bug 325519 Opened 19 years ago Closed 13 years ago

ICS provider provides almost no useful diagnostic info on errors

Categories

(Calendar :: Provider: ICS/WebDAV, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dmosedale, Assigned: Fallen)

References

Details

Attachments

(2 files)

A large proportion of failures in the ICS provider end up being reported as parseICS throwing 0x804a0107.  Unfortunately, this provides no good info to figure out what's really going on.  The onStreamComplete method in the provider needs to check the incoming status, and if its not a success code, it needs to call this.mObserver.onError with an appropriate message, probably including protocol specific information about the failure.  As such, it seems like at least some of this error-handling belongs in the transport hooks defined in bug 324849.
This patch isn't a complete fix for this bug, far from it. But it might make the error dialog slightly less scary by replacing the number with a name. (0x804a0107 becomes ICS_FILE)
Attachment #213188 - Flags: first-review?(jminta)
Comment on attachment 213188 [details] [diff] [review]
shown names, not numbers [checked in]

+    if (aErrNo & calIError.ERROR_BASE) {
+        var err;
+        for (err in calIError) {
Why declare err in a separate line?  If there isn't a reason, then probably move it into the for();

r=jminta

Please consult with dmose before landing.  I'm not sure we want to mess with the error-system at this late stage.
Attachment #213188 - Flags: first-review?(jminta) → first-review+
Comment on attachment 213188 [details] [diff] [review]
shown names, not numbers [checked in]

patch checked in, leaving bug open for the real work.
Attachment #213188 - Attachment description: shown names, not numbers → shown names, not numbers [checked in]
This patch adds two string for some common errors.
Suggestions for more descriptive error string are welcome :)
Assignee: base → mvl
Status: NEW → ASSIGNED
Attachment #215602 - Flags: first-review?(jminta)
1. Suggest using keywords "character encoding" and "iCalendar", such as:

utf8DecodeError = An error occured while decoding the calendar file from UTF-8.  Check that the calendar ICS file is encoded using the UTF-8 character encoding.

icsMalformedError = Parsing the calendar ICS file failed.  Check that the file is a valid iCalendar file.

2. The second string key is mispelled (icsMalformedError vs. icaMalformedError)
Comment on attachment 215602 [details] [diff] [review]
more friendly errors [checked in]

I'd suggest that the UTF8 error message include a very brief comment about what characters are likely to trigger such an error (accented vowel, etc).  That ought to help cut back on the number of "I got this error message, what now?" posts.

gekacheka's comment about iCalendar seems pretty good.  Maybe "... contains valid iCalendar (ics) data..." would keep everybody happy.  I tend to think users are more familiar with the standard as 'ics' rather than the technically more correct 'iCalendar'.

 stillReadOnlyError=There has been an error reading data for calendar: %1$S.
+utf8DecodeError = An error occured while converting the calendar file from utf8. ICS calendar files must always be encoded using utf8.
+icaMalformedError = Parsing the calendar file failed. Check that the file contains valid ics data.
Nit: It looks like current style in these files doesn't put spaces around =.

The UTF8 error is great to have, so r=jminta with whatever tweaks to the error message you feel appropriate after this discussion.

Can we also get these errors into the import code?
Attachment #215602 - Flags: first-review?(jminta) → first-review+
Revised suggestions below.

Revised to include "iCalendar (ics) file" as jminta suggested.

Revised to mention "symbols and accented letters" from suggestion.

As I understand the file cannot be any file that happens to contain some iCalendar data somewhere in the middle (e.g,. a MIME attachment in a message file).  The entire file must be an iCalendar file.  Also, some users may not know what 'valid' means in this context (dates out of order?), so I hope using the word "syntax" helps.  So I changed "file contains valid iCalendar (ics) data" to "file conforms to iCalendar (ics) file syntax".

utf8DecodeError=\
  An error occured while decoding an iCalendar (ics) file as UTF-8.  \
  Check that the file, including symbols and accented letters, \
  is encoded using the UTF-8 character encoding. 

icsMalformedError=\
  Parsing an iCalendar (ics) file failed. \
  Check that the file conforms to iCalendar (ics) file syntax.
Comment on attachment 215602 [details] [diff] [review]
more friendly errors [checked in]

checked in, with the error messages from comment 7. Thanks for the suggestions!
(Leaving bug open for more error work)
Attachment #215602 - Attachment description: more friendly errors → more friendly errors [checked in]
Assignee: mvl → nobody
Status: ASSIGNED → NEW
Component: Internal Components → Provider: ICS/Webdav
QA Contact: base → ics-provider
Version: Trunk → unspecified
Fixed by bug 692755
Assignee: nobody → philipp
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b8
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: