827570 - When downgrading, no local ICS calendars are shown, and no error

Status: NEW

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

Description

[Ben Bucksch (:BenB)]

Reproduction:
- Start build from trunk 2012-10-01 (Oct 1)
- Load some ICS calendar files from harddisk, i.e. file:// URL
- Add some appointments
- Enable "[x] Offline support"
- Close build
- Start ESR17 build

Actual result:
- No errors
- No appointments show up (seems like you have a lot of free time!)
- Error Console (Ctrl-Shift-J) is full of errors

Expected result:
- Calendars load
- If there is any problem for any reason, an error appears which gives
  - precision technical information
  - Consequences (here: "Your appointments cannot be shown") (if known)
  - Suggestions on how to solve it (if known)

Importance:
Critical, because:
- I get no hint that anything is wrong, although things are seriously wrong.
- I can't see my appointments. I can't use the app.

Comment 1

[Ben Bucksch (:BenB)]

Errors - 3 types, each of them repeated many times (I have 4 calendars with many appointments each):

1. Error: [Exception... "'<error>' when calling method: [calICalendar::id]"
nsresult: "0x804a0007 (<unknown>)"  location: "JS frame :: resource://calendar/modules/calUtils.jsm -> .../calendar-js/calCachedCalendar.js :: cCC_setupCachedCalendar :: line 194"  data: no]
Source File: resource://calendar/modules/calUtils.jsm -> .../calendar-js/calCachedCalendar.js Line: 209

2. Error: Assert failed: [Exception... "'[JavaScript Error: "obj is null"
{file: "resource://calendar/modules/calUtils.jsm -> .../calendar-js/calCachedCalendar.js" line: 948}]' when calling method: [calICalendar::getItems]" ... [with stack]

3. Error: this.mCachedCalendar is null
.../calendar-js/calCachedCalendar.js Line: 228

Comment 2

[Ben Bucksch (:BenB)]

Archaeopteryx says: maybe it's this change: http://hg.mozilla.org/comm-central/rev/891e94861a14#l5.40

Comment 3

[Ben Bucksch (:BenB)]

Please note that this bug as 2 components, and thus needs 2 fixes:
- That the calendars don't load
- That no error appears - this must be ensured generically for *any* critical error, e.g. unexpected exceptions

Comment 4

[Stefan Sitter [:ssitter]]

We don't support database downgrading in Lightning. Resolve as Wontfix?

But you can try it manually e.g. by exporting all events to the iCalendar format, remove the calendar database and re-import the calendars once downgraded. And switch off the offline cache before downgrade, remove the cache database and let it recreate by the old Lightning version. Or check your profile for backup copies of the previously used database file before it was upgraded.

Comment 5

[Ben Bucksch (:BenB)]

You have to support downgrades, if you do automatic upgrades. It can't be that starting a newer build just *once* prevents me from ever using the older build again.

Apart from that, please read my description and comment 3 again.

> But you can try it manually

This isn't a support request, I can help myself. End users can't.

Comment 6

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

Did you get any messages along the lines of "upgrading storage format to version XXX" ? This is the type of upgrade we don't support, since it would require us to not only write an SQL upgrader, but also a downgrader. 

Not that writing a downgrader is particularly hard, but we don't have any code patterns in place to downgrade anything. There would need to be changes to calStorageUpgrades.jsm that allow doing this for more than one version and possibly not allowing this if the storage format is very old or it would cause dataloss.

I recall there was another bug around the storage cache that might have caused this error message too. I don't have a reference atm though.

Comment 7

[Ben Bucksch (:BenB)]

Note that my calendars are all in ICS, which I think is a standard that you didn't change. You only changed the format of your local cache. So, you could just drop the cache and refresh it. I think that would have been the right solution for my particular case with ICS calendars.

> Did you get any messages along the lines of "upgrading storage format to version XXX" ?

Not that I remember.

> Not that writing a downgrader is particularly hard

Another way - in general, if this is the main database - is to either
1) amend the format in a backwards-compatible way (preferred way)
2) ask the user for confirmation before upgrading in such a disruptive way
3) Make a backup of the old file, by putting the new format in a new filename
   (plus maybe warn the user, because changes in the new app version won't
   be seen in the old app version)

Comment 8

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

(In reply to Ben Bucksch (:BenB) from comment #7)
> > Did you get any messages along the lines of "upgrading storage format to version XXX" ?
> 
> Not that I remember.
If this was not there, then it might not be a problem of the database upgrade but instead the other bug that recently showed up regarding cached calendars.


> 3) Make a backup of the old file, by putting the new format in a new filename
>    (plus maybe warn the user, because changes in the new app version won't
>    be seen in the old app version)
Oh this should actually be in place, there should be a local.vNN.sqlite or so in your calendar-data directory, and also a cache.vNN.sqlite with NN being the database version.

Comment 9

[Ben Bucksch (:BenB)]

So, that means this should just work when I downgrade? Because the old Lightning version would try to open the old filename, and that still exists, so no failure. Just outdated data. So, that would never cause the bug I saw? Is that what you're saying?

The scheme is pretty smart: With this, you can issue a warning when you downgraded, not when you upgrade: When the code sees a vNN that is *larger* than the one you expect in your code, then that's the moment to warn that the user might be looking at stale data, because all changes made in the newer version won't be seen. But in many cases, that's no problem, because I started the new version only once, and didn't like it - or more likely, it didn't work - and I didn't actually make changes in the new version, so I can happily use the old database. The warning needs to be there, though, in case I did actually use the new version for a while.

Comment 10

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

No, just the files are there for manual copying. Indeed this could be automated a bit more.

Aside from that, we currently don't do user input for the upgraders, since the upgrade is being done the first time a getItems call is done (afaik). What we should probably do is try to move the upgrade to before the main window opens and allow a modal dialog to block starting the main window. I don't know if extensions can still do that after the gecko 2.0 changes.

I guess we could morph this bug into a "support downgrading...somehow" bug, but I can't say when I would get to this. We don't do the storage upgrades very often, so I don't think its currently worth the effort.

Comment 11

[Ben Bucksch (:BenB)]

With what I describe in comment 9, you wouldn't need to do anything on upgrade. Only on downgrade. I guess it ends up being the same code place (getItems()), but would not need to block the processing, just start a warning dialog (which is fire and forget).

Comment 12

[Ben Bucksch (:BenB)]

Just to be sure: Why manual copying? If you're really *using* (not just backing up) file-vNN.type, then the 2 versions can co-exist without manual copying.

Comment 13

[Ben Bucksch (:BenB)]

In other words, the key trick was the second part of the sentence (comment 7):
> 3) Make a backup of the old file, by putting the new format in a new filename