Better handling of server-side deleted calendars when getting the ctag

RESOLVED FIXED in 1.0b1

Status

defect
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: lmarcotte, Assigned: lmarcotte)

Tracking

unspecified
1.0b1

Details

Attachments

(1 attachment, 2 obsolete attachments)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.5) Gecko/2008120121 Firefox/3.0.5
Build Identifier: Lightning 0.9

When getting the ctag of a CalDAV calendar, we might obtain a 404 as the calendar might have been deleted from the server since we last checked for the ctag.

We should better handle this and also, if we even run into this situation (get a 404) and then, get a 207 (since the calendar was re-created), we should also handle this.

I suggest the following modification in safeRefresh(). We currently have :

            try {
                dump("CalDAV: Status " + aContext.responseStatus +
                    " checking ctag for calendar " + thisCalendar.name);
            } catch (ex) {
                dump("CalDAV: Error without status on checking ctag for calendar " +
                    thisCalendar.name);
            }

and we should have:

            try {
                dump("CalDAV: Status " + aContext.responseStatus +
                    " checking ctag for calendar " + thisCalendar.name);
		if (aContext.responseStatus == 404) {
		  LOG("Disabled CalDAV calendar due to 404 error code.");
		  thisCalendar.completeCheckServerInfo(aChangeLogListener, Components.interfaces.calIErrors.DAV_DAV_NOT_CALDAV);
		  return;
		} else if (aContext.responseStatus == 207 && thisCalendar.disabled) {
		  thisCalendar.reenable(aChangeLogListener);
		  return;
		}
            } catch (ex) {
                dump("CalDAV: Error without status on checking ctag for calendar " +
                    thisCalendar.name);
            }

and reenable() should look like:

    reenable: function caldav_reenable(aChangeLogListener) {
      // we reset our calendar status
      this.setProperty("currentStatus", Components.results.NS_OK);
      
      // check if maybe our calendar has become available
      this.checkDavResourceType(aChangeLogListener);
    }

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Whiteboard: [needs patch for code in comment#0]
Posted patch Proposed fix (obsolete) โ€” โ€” Splinter Review
Attachment #363576 - Flags: review?(philipp)
Assignee: nobody → lmarcotte
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [needs patch for code in comment#0]
Attachment #363576 - Flags: review?(philipp) → review-
Comment on attachment 363576 [details] [diff] [review]
Proposed fix

I'd prefer having currentStatus be reset when checkDavResourceType actually succeeds. If we do it as you proposed, then the status icon will go away, then come back again if the calendar is still gone missing.


maybe the solution would be to set currentStatus = NS_OK in the first chunk of your patch (instead of calling reenable), and have checkDavResourceType take care of also setting it to NS_OK if everything succeeds.

r- for now.
Posted patch WiP Patch - v2 (obsolete) โ€” โ€” Splinter Review
Ludovic, this can serve as a base for your new patch. Please do some testing to see if it makes sense this way (with and without the calendar being cached).
Attachment #363576 - Attachment is obsolete: true
Attachment #363588 - Flags: review?(lmarcotte)
Posted file Philip's patch, reviewed. โ€”
Attachment #375380 - Flags: review?(philipp)
Attachment #363588 - Attachment is obsolete: true
Attachment #363588 - Flags: review?(lmarcotte)
Attachment #375380 - Flags: review?(philipp) → review+
Comment on attachment 375380 [details]
Philip's patch, reviewed.

Looks good, r=philipp
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/3c26a22f1549>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.