327971 - schema version checker failure blows away existing db tables

Status: RESOLVED FIXED

(Calendar :: Provider: Local Storage, defect)

Description

[OOO until Apr 20; Dan Mosedale (:dmosedale, :dmose)]

If the schema version checker for calStorageProvider fails for some unknown reason, the provider will attempt to drop all the tables and start over.  It should really just use the error-announcer to notify the user.

Comment 1

[OOO until Apr 20; Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch, v1

Up until now, versionCheck was this weird combo function that returned magic constants to indicate errors and used those to decide whether to try and initialize the database.  I changed the calling code to explicitly look for a version table as a mechanism to decide that, and renamed the function to getVersion, made it only return the version number, and throw exceptions in case of trouble.

Comment 2

[Joey Minta]

Comment on attachment 213012 [details] [diff] [review]
patch, v1

-            selectSchemaVersion.reset();
+            dump ("++++++++++++ DB Error: " + this.mDB.lastErrorString + "\n");
As we discussed in irc, this might be superfluous.  If you want to keep it, at least identify the code area where you're dumping this from.  Something like "getVersion() DB Error:"

Just a reminder to test to make sure we don't need read-only mode here, since createCalendar() ought to fail.

r=jminta with that.

Comment 3

[OOO until Apr 20; Dan Mosedale (:dmosedale, :dmose)]

Turns out createCalendar does fail, but getCalendars eats that failure (bug 328617 filed) and it doesnt propagate to the calling code.  Since getCalendars is what is used to create the error announcers, this means that there is no announcer for the calendar in question for us to use to display this to the user.  However, an exception does show up in the JS console.  Since the behavior with this patch is much, much better than the behavior without this patch, Ive filed bug 328618 to deal with that.  Im not sure whether 328618 should block 0.1, though I kind of suspect not.

Comment 4

[OOO until Apr 20; Dan Mosedale (:dmosedale, :dmose)]

Changed the dump() as suggested and landed this fix.  Lets continue any error-handling discussion in bug 328618.