Closed Bug 890217 Opened 7 years ago Closed 2 years ago

Sync fails partially (bookmarks fail, tabs & history seem to synchronise)

Categories

(Firefox :: Sync, defect, P1)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox58 --- fixed

People

(Reporter: v.solnicky, Assigned: Lina)

Details

(Whiteboard: [sync:scale][sync:bookmarks])

Attachments

(2 files)

My set-up:

One Windows XP computer with Seamonkey 2.19 & Firefox, both syncing fine.
One Windows XP laptop with Firefox, syncing with all the  above fine.
One CentOS 6 on x86_64 with Seamonkey 2.19 having the problem described below [Mozilla/5.0 (X11; Linux x86_64; rv:22.0) Gecko/20100101 Firefox/22.0 SeaMonkey/2.19] -- but the problem is new, it persists through all versions of Seamonkey since June 2012, i.e. including 2.17.0 and 2.17.1
The same CentOS 6 box with current CentOS 6 repository Firefox (probably 17.x -- long support life edition) syncing without any problems.

The Linux Seamonkey has never synchronised bookmarks since I set up synchronisation a year ago, it has been synchronising tabs and history successfully. Recently I have found the way to get logs <about:sync-log> ... they show that the sync tries to upload the initial bunch of local bookmarks and fails:

[...]
1372879930578    Sync.Engine.Bookmarks    DEBUG    First sync, uploading all items
1372879931575    Sync.Tracker.Clients    DEBUG    Saving changed IDs to clients
1372879931801    Sync.Engine.Bookmarks    INFO    3149 outgoing items pre-reconciliation
1372879932801    Sync.Tracker.Bookmarks    DEBUG    Saving changed IDs to bookmarks
1372879935758    Sync.SyncScheduler    DEBUG    Next sync in 90000 ms.
1372879938937    Sync.Engine.Bookmarks    WARN    Got exception "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsINavBookmarksService.getBookmarkURI] Stack trace: _buildGUIDMap()@resource://gre/modules/services-sync/engines/bookmarks.js:246 < resource://gre/modules/services-sync/engines/bookmarks.js:357 < _mapDupe()@resource://gre/modules/services-sync/engines/bookmarks.js:313 < _findDupe()@resource://gre/modules/services-sync/engines/bookmarks.js:413 < _reconcile()@resource://gre/modules/services-sync/engines.js:1121 < resource://gre/modules/services-sync/engines.js:904 < resource://gre/modules/services-sync/record.js:625 < Channel_onDataAvail()@resource://gre/modules/services-sync/resource.js:542 < <file:unknown>" building GUID map. Skipping all other incoming items.
1372879938937    Sync.Engine.Bookmarks    WARN    Reconciliation failed: aborting incoming processing.

[...]

1372879959029    Sync.Status    DEBUG    Status.service: success.status_ok => error.sync.failed_partial
1372879959029    Sync.ErrorHandler    DEBUG    bookmarks failed: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsINavBookmarksService.getBookmarkURI] Stack trace: _buildGUIDMap()@resource://gre/modules/services-sync/engines/bookmarks.js:246 < resource://gre/modules/services-sync/engines/bookmarks.js:357 < _mapDupe()@resource://gre/modules/services-sync/engines/bookmarks.js:313 < _findDupe()@resource://gre/modules/services-sync/engines/bookmarks.js:413 < _reconcile()@resource://gre/modules/services-sync/engines.js:1121 < resource://gre/modules/services-sync/engines.js:904 < resource://gre/modules/services-sync/record.js:625 < Channel_onDataAvail()@resource://gre/modules/services-sync/resource.js:542 < <file:unknown>

[...]

1372879960366    Sync.Synchronizer    INFO    Sync completed at 2013-07-03 21:32:40 after 30.26 secs.
1372879960366    Sync.SyncScheduler    DEBUG    Next sync in 600000 ms.
1372879960366    Sync.ErrorHandler    DEBUG    Some engines did not sync correctly.

The bookmarks originated from a Seamonkey 1.x salvaged profile that was migrated to a SM 2.x profile using its built-in mechanism. Some the size may be a bit large.

I am attaching the full log, with just my login name removed (replaced with MyLogin).
There's a record in your bookmark database which claims to be a bookmark, but doesn't have a URI.

(Similar to Bug 821757, which reports a malformed URI.)

So you need to find the invalid entries in your 3,149-bookmark database and fix or remove them.

mak, are there any tools for validating the *content* of a places DB, not just the structure and DB elements?
Severity: major → normal
Flags: needinfo?(mak77)
Whiteboard: [sync:scale][sync:bookmarks]
Thanks for the prompt reaction. I have tried to install SQLite Manager <https://addons.mozilla.org/cs/seamonkey/addon/sqlite-manager/?src=ss> (not really friendly for SeaMonkey, but works) and if and only if
1) The right data-base be "places.sqlite" and
2) the right table be "moz_places" and
3) the right column be "url", then
then using the search ability I cannot find any records with url = nil, url = "" or url starting with " " (space). I have also troed the following sql queries with the same result (no error, but no records returned) -- please note that I am not an SQL guru:
SELECT * FROM moz_places where url IS NULL;
SELECT * FROM moz_places where url = NULL;
SELECT * FROM moz_places where url = "";
SELECT * FROM moz_places where url = " %";
SELECT * FROM moz_places where url like " %";

So do you have any suggestion either where I am making a mistake in queries or what else can be wrong, please?

Thanks in advance!
(In reply to Richard Newman [:rnewman] from comment #1)
> mak, are there any tools for validating the *content* of a places DB, not
> just the structure and DB elements?

If that seamonkey is updated enough it should have maintenance integrated, you can try evaluating this in the error console

Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");PlacesDBUtils.maintenanceOnIdle();
Flags: needinfo?(mak77)
the right query is a bit more complicated, something like
SELECT b.id FROM moz_bookmarks b JOIN moz_places h ON b.fk = h.id WHERE url ISNULL
should give the bookmark id in moz_bookmarks that is pointing to a null url entry.
Did you ever run manual UPDATE or INSERT queries on that database?
hm, or maybe just
SELECT id FROM moz_bookmarks WHERE type = 1 and fk ISNULL
The other query is unlikely to return anything  since you didn't find anything in comment 2
1) The Seamonkey is 2.19, offered to update a day ago (from the last version before, 2.17.1).

2) SELECT id FROM moz_bookmarks WHERE type = 1 and fk ISNULL
returns no rows with "Last error: not an error" for DB "Direcory > places.sqlite".

3) SELECT b.id FROM moz_bookmarks b JOIN moz_places h ON b.fk = h.id WHERE url ISNULL seems to return the same -- no rows, no error.
Try running this in a Browser Console (or privileged Web Console, if SM doesn't have Browser Console yet):

Components.utils.import("resource://services-sync/main.js");
let bme = Weave.Service.engineManager.get("bookmarks");
let ids = Object.keys(bme._store.getAllIDs());
for each (let id in ids) {
  try {
    bme._store.createRecord(id, "bookmarks");
  } catch (e) {
    console.log("Failing record: " + id);
  }
}

Then you can look in the DB and see which bookmark has that GUID.
fwiw, also comment 3 is still a valid option.
Result from running "Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm"); PlacesDBUtils.maintenanceOnIdle();" in the Error Console -- after pressing Evaluate it does nothing, shows nothing, (show all is set). Regarding Comment 7, I do not know what is privileged web console and how to run it. This compilation of SM seems to have only error console and Venkman (JS debbuger -- but I have never used it). If I try to run it error console, I got error that let is a reserved identifier and if I remove it, I get another error. Can you advise, please?

I may be able to see my computer only once a day this Friday-Sunday , so I apologise for possible late responses. Thanks.
(In reply to Vladimír Solnický from comment #9)
> (show all is set).

what does this mean?

> Regarding
> Comment 7, I do not know what is privileged web console and how to run it.

It's the error console, though it doesnt support all of the js constructs cause it support js 1.5 but none of the features introduced in later js versions, so for example instead of "let" you should use "var".
(In reply to Vladimír Solnický from comment #9)

> Comment 7, I do not know what is privileged web console and how to run it.

It's a console that allows you to poke at the browser, rather than just the current page.

Open a new tab. Open about:config. Then open the Error Console.

Run the provided code in there.
(In reply to Marco Bonardo [:mak] from comment #10)
> (In reply to Vladimír Solnický from comment #9)
> > (show all is set).
> 
> what does this mean?

That no viewing filter was in place. I just tried to save time in case you asked about this.

> > Regarding
> > Comment 7, I do not know what is privileged web console and how to run it.
> 
> It's the error console, though it doesnt support all of the js constructs
> cause it support js 1.5 but none of the features introduced in later js
> versions, so for example instead of "let" you should use "var".

I've done so (incorporating hints from comment 11) and it still looks like the js in error console is not supporting "for each" construction -- I get "missing (" and when I added it requires semicolon. I've done more tests, the code I've finished with is here:

Components.utils.import("resource://services-sync/main.js"); 
var bme = Weave.Service.engineManager.get("bookmarks"); 
var ids = Object.keys(bme._store.getAllIDs()); 
var id; 
for( each id in ids ) { 
  try { 
    bme._store.createRecord(id, "bookmarks"); 
  } catch (e) { 
    console.log("Failing record: " + id); 
  } 
}

:-(
this cannot work:
for( each id in ids ) { 
you may rather want:
ids.forEach(function (id) {
   try { 
    bme._store.createRecord(id, "bookmarks");
  } catch (e) { 
    console.log("Failing record: " + id); 
  }
});
I have to do something incorrectly. I used the following code:

Components.utils.import("resource://services-sync/main.js"); 
var bme = Weave.Service.engineManager.get("bookmarks"); 
var ids = Object.keys(bme._store.getAllIDs()); 
var id; 
ids.forEach(function (id) {
   try { 
    bme._store.createRecord(id, "bookmarks");
  } catch (e) { 
    console.log("Failing record: " + id); 
  }
});
console.log("THE END");  

I would expect at least the text "THE END" be output to the console -- nothing is there. I would expect the load to be high if the evaluation takes too much time -- but no, the load of one core is 100 % for about two seconds (and SM is the process using it) and then -- finito, no load ... and no output. :-(
I just spent some time experimenting. Apparently Seamonkey's error console is decrepit, and only supports reportError.

Try this:

Components.utils.import("resource://services-sync/main.js"); 
var bme = Weave.Service.engineManager.get("bookmarks"); 
var ids = Object.keys(bme._store.getAllIDs());
Components.utils.reportError("Beginning bookmark walk...");
ids.forEach(function (id) {
   try { 
    bme._store.createRecord(id, "bookmarks");
  } catch (e) { 
    Components.utils.reportError("Failing record: " + id); 
  }
});
Components.utils.reportError("Done.");
Seamonkey uses the Error console, it's the same Firefox had until we switched to the Browser Console not too much time ago.
Thank you very much, this new code works perfectly. I may also have searched in a wrong table. Now I found two records marked as invalid. None of them has empty URL, but both has a URI with protocol imap: -- meaning this is a SeaMonkey specific thing as Firefox doesn't support this protocol (I expect). This probably originated from the following behaviour pattern -- I used to bookmark all open tabs when I was going to do something I expected may have crashed SM or when I had to interrupt my work. And as opening an image from IMAP attachment is being done in a new tab in SM, some really interesting images (at that time) were bookmarked.

Do you think this SM-specific protocol can be included or I should remove both records? And in case the latter is true, should I remove related records from both tables (moz_bookmarks and moz_places)?

Thanks for all your help!

Have a nice day.
ah yes, Firefox history refuses to store imap urls. Seamonkey uses the same protocol, though. What I don't understand is why it fails to create a bookmark to this scheme, I can't find any point in bookmarks code where we disallow that, indeed bookmarks don't make any check on protocols, only history does.
So, I don't understand why createRecord fails.
could you please in the catch do a
Components.utils.reportError(new Error(e).stack);
and report here the stack you get?
(In reply to Marco Bonardo [:mak] from comment #18)
> What I don't understand is why it fails to create a
> bookmark to this scheme, I can't find any point in bookmarks code where we
> disallow that, indeed bookmarks don't make any check on protocols, only
> history does.
> So, I don't understand why createRecord fails.

To be clear: bookmarkService.getBookmarkURI() throws. Sync's "createRecord" should really be called "build an in-memory representation of the Places content with this ID"; it doesn't do any writing.

The problem appears to be that getBookmarkURI is doing more validation than whatever part of Seamonkey was responsible for storing the IMAP bookmark.

Is it possible that

 if (bookmark.type == TYPE_BOOKMARK) {
    // If not a tag, recalculate frecency for this entry, since it changed.
    if (bookmark.grandParentId != mTagsRoot) {
      nsNavHistory* history = nsNavHistory::GetHistoryService();
      NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
      rv = history->UpdateFrecency(bookmark.placeId);
      NS_ENSURE_SUCCESS(rv, rv);
    }

is causing the issue, if you say that history does protocol checking? After all, it's either FetchItemInfo or NS_NewURI that's failing, AFAICS…
(In reply to Richard Newman [:rnewman] from comment #20)
> To be clear: bookmarkService.getBookmarkURI() throws.

Interesting, getBookmarkURI doesn't make any special checks

>  if (bookmark.type == TYPE_BOOKMARK) {
>     // If not a tag, recalculate frecency for this entry, since it changed.
>     if (bookmark.grandParentId != mTagsRoot) {
>       nsNavHistory* history = nsNavHistory::GetHistoryService();
>       NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
>       rv = history->UpdateFrecency(bookmark.placeId);
>       NS_ENSURE_SUCCESS(rv, rv);
>     }
> 
> is causing the issue, if you say that history does protocol checking? After
> all, it's either FetchItemInfo or NS_NewURI that's failing, AFAICS…

updateFrecency would throw only if the passed-in is invalid.

So, in bookmarks the only thing that seem could fail is NS_NewURI, or bookmark.placeId is wrong.

We need the url to be able to verify if it's a not valid URI (but then I'd wonder how it entered the database)
(In reply to Marco Bonardo [:mak] from comment #19)
> could you please in the catch do a
> Components.utils.reportError(new Error(e).stack);
> and report here the stack you get?

The first record:
Timestamp: 8.7.2013 22:06:43
Error: @javascript:%20Components.utils.import("resource://services-sync/main.js");%20%20var%20bme%20=%20Weave.Service.engineManager.get("bookmarks");%20%20var%20ids%20=%20Object.keys(bme._store.getAllIDs());%20Components.utils.reportError("Beginning%20bookmark%20walk...");%20ids.forEach(function%20(id)%20{%20%20%20%20try%20{%20%20%20%20%20%20bme._store.createRecord(id,%20"bookmarks");%20%20%20}%20catch%20(e)%20{%20%20%20%20%20%20Components.utils.reportError(new%20Error(e).stack);%20%20%20}%20});%20Components.utils.reportError("Done.");:1
@javascript:%20Components.utils.import("resource://services-sync/main.js");%20%20var%20bme%20=%20Weave.Service.engineManager.get("bookmarks");%20%20var%20ids%20=%20Object.keys(bme._store.getAllIDs());%20Components.utils.reportError("Beginning%20bookmark%20walk...");%20ids.forEach(function%20(id)%20{%20%20%20%20try%20{%20%20%20%20%20%20bme._store.createRecord(id,%20"bookmarks");%20%20%20}%20catch%20(e)%20{%20%20%20%20%20%20Components.utils.reportError(new%20Error(e).stack);%20%20%20}%20});%20Components.utils.reportError("Done.");:1

Source File: javascript:%20Components.utils.import("resource://services-sync/main.js");%20%20var%20bme%20=%20Weave.Service.engineManager.get("bookmarks");%20%20var%20ids%20=%20Object.keys(bme._store.getAllIDs());%20Components.utils.reportError("Beginning%20bookmark%20walk...");%20ids.forEach(function%20(id)%20{%20%20%20%20try%20{%20%20%20%20%20%20bme._store.createRecord(id,%20"bookmarks");%20%20%20}%20catch%20(e)%20{%20%20%20%20%20%20Components.utils.reportError(new%20Error(e).stack);%20%20%20}%20});%20Components.utils.reportError("Done.");
Line: 1

The second record:
Timestamp: 8.7.2013 22:06:45
Error: @javascript:%20Components.utils.import("resource://services-sync/main.js");%20%20var%20bme%20=%20Weave.Service.engineManager.get("bookmarks");%20%20var%20ids%20=%20Object.keys(bme._store.getAllIDs());%20Components.utils.reportError("Beginning%20bookmark%20walk...");%20ids.forEach(function%20(id)%20{%20%20%20%20try%20{%20%20%20%20%20%20bme._store.createRecord(id,%20"bookmarks");%20%20%20}%20catch%20(e)%20{%20%20%20%20%20%20Components.utils.reportError(new%20Error(e).stack);%20%20%20}%20});%20Components.utils.reportError("Done.");:1
@javascript:%20Components.utils.import("resource://services-sync/main.js");%20%20var%20bme%20=%20Weave.Service.engineManager.get("bookmarks");%20%20var%20ids%20=%20Object.keys(bme._store.getAllIDs());%20Components.utils.reportError("Beginning%20bookmark%20walk...");%20ids.forEach(function%20(id)%20{%20%20%20%20try%20{%20%20%20%20%20%20bme._store.createRecord(id,%20"bookmarks");%20%20%20}%20catch%20(e)%20{%20%20%20%20%20%20Components.utils.reportError(new%20Error(e).stack);%20%20%20}%20});%20Components.utils.reportError("Done.");:1

Source File: javascript:%20Components.utils.import("resource://services-sync/main.js");%20%20var%20bme%20=%20Weave.Service.engineManager.get("bookmarks");%20%20var%20ids%20=%20Object.keys(bme._store.getAllIDs());%20Components.utils.reportError("Beginning%20bookmark%20walk...");%20ids.forEach(function%20(id)%20{%20%20%20%20try%20{%20%20%20%20%20%20bme._store.createRecord(id,%20"bookmarks");%20%20%20}%20catch%20(e)%20{%20%20%20%20%20%20Components.utils.reportError(new%20Error(e).stack);%20%20%20}%20});%20Components.utils.reportError("Done.");
Line: 1
Vladimír, can you provide the two failing imap: URIs?
(In reply to Marco Bonardo [:mak] from comment #21)
> We need the url to be able to verify if it's a not valid URI (but then I'd
> wonder how it entered the database)

The URLs are:
<imap://eleven.vs.solnicky.cz:993/fetch%3EUID%3E/CURRENT%3E2433?part=1.2&type=text/html&filename=TomEdwards.html>
<imap://vs@eleven.vs.solnicky.cz:993/fetch%3EUID%3E/INBOX%3E56291?part=1.2&type=image/jpeg&filename=invalidazPrahy.jpg>

Hope it helps.
(In reply to Richard Newman [:rnewman] from comment #23)
> Vladimír, can you provide the two failing imap: URIs?

Just done in parallel. :-)
To let you know --
I have backed up the original DB and the records and removed the offending records from moz_bookmarks. Then the sync has started to work (fist time with some errors and a lot of reconciliation, second time without any errors).

Do you think that the imap protocol is going to be allowed for SM and Thunderbird in the future so I can return the records back?
As I said above, the imap is already allowed, I don't have a clue on the error so far, but I guess we can make local testing to see what happens.
I tried to create a bookmark with the same uris you posted in comment 24, and then executed the code in comment 15, I don't see any failure. Maybe those had something else special.
Vladimír: would you be able to share the entire bookmark and place row for one of those records? Apparently it's not just the URI at fault.
Yes, here is it (every record is one line, line breaks have been inserted by the browser):

moz_bookmarks:
INSERT INTO "someTable" VALUES ("1634","1","1840","1618","15","invalidazPrahy.jpg (JPEG Image, 1280x1024 pixels) - Scaled (71%)",null,null,"1176750791000000","1339234897288471","0JE5dnH_zvvZ");

"1634","1","1840","1618","15","invalidazPrahy.jpg (JPEG Image, 1280x1024 pixels) - Scaled (71%)",,,"1176750791000000","1339234897288471","0JE5dnH_zvvZ"

moz_places:
INSERT INTO "someTable" VALUES ("1840","imap://vs@eleven.vs.solnicky.cz:993/fetch%3EUID%3E/INBOX%3E56291?part=1.2&type=image/jpeg&filename=invalidazPrahy.jpg",null,"zc.ykcinlos.sv.nevele.","0","0","0",null,"20",null,"1z67bgoY8Itv");

"1840","imap://vs@eleven.vs.solnicky.cz:993/fetch%3EUID%3E/INBOX%3E56291?part=1.2&type=image/jpeg&filename=invalidazPrahy.jpg",,"zc.ykcinlos.sv.nevele.","0","0","0",,"20",,"1z67bgoY8Itv"
I don't see anything strange yet, I added both to a test database and getBookmarkURI has no issue with it, I tried with both a valid and an invalid parent, no issues.
As well as the snippet in comment 15 does not throw and completes correctly.
Still no clue. Richard, any idea?
No idea, but I'll flag myself to think about it...
Flags: needinfo?(rnewman)
Flags: needinfo?(rnewman)
Flags: needinfo?(kit)
We switched to the async bookmarks API in bug 1274108, and I just tested that the IMAP URLs in comment 29 can be added and retrieved. I uploaded a patch with the test, but I don't think there's anything else actionable here.
Flags: needinfo?(kit)
Comment on attachment 8920667 [details]
Bug 890217 - Add a test for syncing bookmarks with IMAP URLs.

https://reviewboard.mozilla.org/r/191664/#review196894

LGTM!
Attachment #8920667 - Flags: review?(tchiovoloni) → review+
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06cfb3a42a3f
Add a test for syncing bookmarks with IMAP URLs. r=tcsc
Assignee: nobody → kit
Status: NEW → ASSIGNED
Priority: -- → P1
https://hg.mozilla.org/mozilla-central/rev/06cfb3a42a3f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.