312883 - ICS provider backup failure in nsIFile.copyTo

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

I've now seen this on both Windows and Mac.

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80520008
(NS_ERROR_FILE_ALREADY_EXISTS) [nsIFile.copyTo]"  nsresult: "0x80520008
(NS_ERROR_FILE_ALREADY_EXISTS)"  location: "JS frame ::
file:///Users/dmose/s/aviary-trunk/mozilla/obj-tb/dist/xpi-stage/lightning/components/calICSCalendar.js
:: anonymous :: line 318"  data: no]
************************************************************

In addition to fixing this failure, I think we need to put the file operations
in a try block so that we can notify the user of an error and continue to save
the file, even if the backup fails.

Comment 1

[Stefan Sitter [:ssitter]]

For your information: 

I get the same error message when following the steps from https://bugzilla.
mozilla.org/show_bug.cgi?id=302158#c6. At least at the first time.

After a restart of Sunbird these steps crash Sunbird as described in 302158.

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: remove before copying

This patch removes the old file before making a new copy. It might give
dataloss if something goes wrong at the wrong moment, but given that this is a
backup of a backup, i don't think it is worth fixing.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 200361 [details] [diff] [review]
remove before copying

>Index: providers/ics/calICSCalendar.js
>===================================================================
>RCS file: /cvsroot/mozilla/calendar/providers/ics/calICSCalendar.js,v
>retrieving revision 1.29
>diff -u -9 -p -d -r1.29 calICSCalendar.js
>--- providers/ics/calICSCalendar.js	19 Oct 2005 17:54:43 -0000	1.29
>+++ providers/ics/calICSCalendar.js	21 Oct 2005 17:01:47 -0000
>@@ -222,18 +222,34 @@ calICSCalendar.prototype = {
>             }
> 
>             if (doDailyBackup)
>                 purgeBackupsByType(files, 'day');
>             else
>                 purgeBackupsByType(files, 'edit');
> 
>             return;
>         }
>+        
>+        function copyToOverwriting(oldFile, newParentDir, newName) {
>+            var newFile = newParentDir.clone();
>+            newFile.append(newName);
>+            

How about moving the above two statements inside the try block, just in case?

>+            try {
>+                if (newFile.exists()) {
>+                    newFile.remove(false);
>+                }
>+                oldFile.copyTo(newParentDir, newName);
>+            } catch(e) {
>+                dump("Backup failed, no copy:"+e+"\n");
>+                // Error in making a daily/initial backup.
>+                // not fatal, so just continue

Using Components.utils.reportError instead of dump() would make this stuff show
up in the console even in release builds.

r=dmose, with or without the above changes

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in