960816 - FxA-enabled sync still uses old-style node re-assignment flow

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Ryan Kelly [:rfkelly]]

Attachment: sync-log.txt

Running Nightly [29.0a1 (2014-01-16)] I managed to get from from FxA account to a clusterURL in our existing phoenix datacentre, which of course doesn't like my FxA-provided credentials.

Sync debug log attached.

It looks like the flow went like this:

  * authed successfully, hit my new experimental tokenserver, was assigned to https://sync1.dev.lcip.org

  * the url given by the new tokenserver was busted, so it got a 404:
    "mesg: GET fail 404 https://sync1.dev.lcip.org/1.1/8/info/collections"

  * this triggered the old-style node lookup code e.g. "Finding cluster for user 8"

  * the existing node-allocation servers happily assigned me a node in phx

  * the phx node failed with a 401 for obvious reasons

  * I got the "password incorrect" screen.

A syncstorage failure like this should trigger it to go back to the tokenserver to check its clusterURL, not to the old-sync node servers.

Comment 1

[Richard Newman [:rnewman]]

Ooh, exciting! There are probably going to be more of these.

Comment 2

[James Bonacci [:jbonacci]]

We need to triage this on desktop and Android.
So, I added a bunch of people...

Comment 3

[James Bonacci [:jbonacci]]

Adding both Meta bugs for now. We can split off if this needs to be addressed cross-platform.

Comment 4

[Richard Newman [:rnewman]]

Android shouldn't be affected -- it has a separation between stages. Important to test, though.

Comment 5

[Ryan Kelly [:rfkelly]]

>  * the url given by the new tokenserver was busted, so it got a 404:

Actually no, the tokenserver gave the right URL.  The client stripped off the path component:

"1389916731312	Sync.BrowserIDManager	INFO	initWithLoggedUser has username 8, endpoint is https://sync1.dev.lcip.org/"

I guess the client hard-codes the structure of the path component, and just appends "/1.1/username/" to its clusterURL.  The new servers are expected "/1.5/username/" per new protocol version.

Comment 6

[Nick Alexander :nalexander [he/him]]

Ditto what rnewman said.  If this was busted, we'd have seen it already for each new Account created.

Comment 7

[Edwin Wong [:edwong]]

This class of bugs underscores the requirement for using production servers when we go live in nightly.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

The patch in bug 959222 will ensure we don't use the old-sync node re-assignment flow.  However, it doesn't yet allow us to support version 1.5 of the token server, which we will track in a different bug

Comment 9

[Mark Hammond [:markh] [:mhammond]]

I believe bug 959222 landed support for this.