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

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Ludovic Marcotte]

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.

Comment 1

[Ludovic Marcotte]

Attachment: Proposed fix

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: WiP Patch - v2

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).

Comment 4

[Ludovic Marcotte]

Attachment: Philip's patch, reviewed.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 375380 [details]
Philip's patch, reviewed.

Looks good, r=philipp

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/3c26a22f1549>

-> FIXED