"Unable to process the backup file" when importing bookmarks from JSON backup, but the bookmarks are imported correctly

NEW
Unassigned

Status

()

Firefox
Bookmarks & History
P2
normal
a year ago
a year ago

People

(Reporter: StefanG_QA, Unassigned)

Tracking

53 Branch
Unspecified
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox53 affected)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Created attachment 8828919 [details]
about sync shows "Unable to process backup file" when the file is processed correctly.png

Mozilla/5.0(Macintosh; Intel Mac OS X 10.11; rv:53.0)Gecko/20100101 Firefox/53.0

Precondition:
Download AboutSync addon: https://addons.mozilla.org/en-US/firefox/addon/about-sync/

Step
1. Launch firefox with an existing sync account
2. Go to Menu Bar > Bookmarks > Show all bookmarks
3. Click on Backup from "Import and Backup" drop down list
4. Save Bookmarks as Json file
5. Add, Edit, Delete bookmarks from the account profile, trigger sync and wait for finish
6. In About Sync, verify that there are no bookmarks missing from the server
7. Close and reopen Firefox with the same profile
8. Go to Menu Bar > Bookmarks > Show all bookmarks
9. Click on "Import and Backup" and select Restore -> Choose file
10. Select backed up file from step 4 to restore
11. Click Ok
12. Trigger Sync and wait for it to finish
13. In About Sync, verify that there are no bookmarks missing from the server

AR: About:Sync shows wrong message "Unable to process backup file" while the bookmarks are imported correctly (see the attached screenshot)

ER: The backup file is imported correctly, About:Sync show successful message

Comment 1

a year ago
I encountered this issue too one time on Windows 10. 
When I tried second time to restore same bookmarks json file, it got imported without any issue.
Note: Backup json file is from some other old profile.

Updated

a year ago
Flags: needinfo?(mark.page)
It's not clear to me what About Sync has to do with this. From the screenshot, it looks like that's the Library UI, and About Sync isn't involved at all? The code that shows the error is at http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/browser/components/places/content/places.js#515-521, but we don't log why `BookmarkJSONUtils.importFromFile` threw.

Mak, do you think it's worth adding a `Cu.reportError` call here? It's also interesting that Stefan and Kanchan's bookmarks were imported successfully, despite the error, and that importing again makes the error go away.
Component: WebExtensions: General → Bookmarks & History
Flags: needinfo?(mark.page) → needinfo?(mak77)
Product: Toolkit → Firefox
Summary: About Sync shows "Unable to process the backup file" message when importing bookmarks from JSON backup file while the file is imported correctly → "Unable to process the backup file" when importing bookmarks from JSON backup, but the bookmarks are imported correctly
(In reply to Kit Cambridge [:kitcambridge] from comment #2)
> involved at all? The code that shows the error is at
> http://searchfox.org/mozilla-central/rev/
> 30fcf167af036aeddf322de44a2fadd370acfd2f/browser/components/places/content/
> places.js#515-521, but we don't log why `BookmarkJSONUtils.importFromFile`
> threw.
> 
> Mak, do you think it's worth adding a `Cu.reportError` call here?

That method uses importFromFile that already has a reportError:
Cu.reportError("Failed to restore bookmarks from " + aFilePath + ": " + ex);

It would be useful to know what it printed to the console.

> It's also
> interesting that Stefan and Kanchan's bookmarks were imported successfully,
> despite the error, and that importing again makes the error go away.

It depends on what we mean by imported successfully, it's possible the process worked 99%, but missed something that may not be easily detectable through the ui. For example, it may have failed to create a keyword? OR maybe something was just busy and the first time it threw? Many things may go wrong without being clearly visible in the UI.
Flags: needinfo?(mak77)
stefan, could you please report the Browser console output?
Flags: needinfo?(stefan.georgiev)
(Reporter)

Comment 5

a year ago
Created attachment 8830101 [details]
Importing Bookmarks 'Failed'  browser console outoput.txt

Please find the requested Browser Console output as an attached txt file
Flags: needinfo?(stefan.georgiev)
UNIQUE constraint failed: moz_keywords.keyword  BookmarkJSONUtils.jsm:111
Looks like the restore tries to recreate an already existing keyword.
I think we may want to ignore keyword insertion errors, regardless "Unable to process backup file" is not a good error to show to the user for this case. We should probably differentiate a complete failure (invalid file) from a partial failure ("We were unable to import some of the items").
This bug could cover figuring out a better error message.

That said, there is probably a race condition here. We remove all the bookmarks to start restoring, keywords observe bookmarks removal and start their removal. The restore process may begin before the keywords work is done, and then effectively we could miss restoring a keyword because the insertion fails and there's a pending removal.
We could:
1. find a way to wait for pending keywords work, but that sounds hackish.
2. explicitly remove all the keywords before starting removing bookmarks. The problem is that once keywords are decoupled from bookmarks (bug 648398), this may end up removing too much.
3. in the keywords bookmarks observer abort a keyword deletion if in the meanwhile someone tries to insert the same keyword OR if there are pending observer operations yield on those before doing the insertion.

Solution 3 looks like the best one, so far.
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.