1212426 - Use X-If-Unmodified-Since=0 to make ServerConfigurationRequiredError robust

Status: NEW

(Firefox for iOS :: Sync, defect)

Description

[Nick Alexander :nalexander [he/him]]

Right now, ServerConfigurationRequiredError does two sequential PUTs to upload meta/global and crypto/keys.  We can do better: we should use X-I-U-S=0 and add a terminal ClientRaceDetected state when we see 412 status codes back.

Comment 1

[Richard Newman [:rnewman]]

Nick, can you mentor this, perhaps for Steph, James, or Emily?

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #1)
> Nick, can you mentor this, perhaps for Steph, James, or Emily?

Yes, absolutely.  Details on X-If-Unmodified-Since and 412 responses can be found at http://docs.services.mozilla.com/storage/apis-1.5.html#request-headers.

The code change required is including ifUnmodifiedSince: 0 to the two relevant PUTs at https://github.com/mozilla/firefox-ios/blob/c4fb24a4b4345943022db5b115811362e57f2223/Sync/SyncStateMachine.swift#L407 and just below it.  (Those two can be simultaneous, BTW.)  In success cases, advance as before.

The error case is when you see a 412.  Handle that by adding a new terminal state (say ClientRaceDetected) to the SyncStateMachine, like ClientUpgradeRequired.  (See https://people.mozilla.org/~nalexander/SyncStateMachine.pdf and https://people.mozilla.org/~nalexander/SyncStateMachine.graffle.)  The terminal state should just fail; next sync, we'll try again.

The hardest part of this will be testing, since MockSyncServer doesn't handle X-I-U-S for individual PUT requests.  And since it will probably take a custom subclass to arrange for the desired 412 Conflict response.

Comment 3

[Stephan Leroux [:sleroux]]

1.2?

Comment 4

[Richard Newman [:rnewman]]

Yes, if Nick can mentor it in the right timeframe.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Does this need to go in 1.3?

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Stefan Arentz [:st3fan] from comment #5)
> Does this need to go in 1.3?

No.  Tracking+, whatever that is for iOS.

Comment 7

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Nick, Richard, what do we need to do with this bug? It keeps popping up in triage.

Comment 8

[Nick Alexander :nalexander [he/him]]

(In reply to Stefan Arentz [:st3fan] from comment #7)
> Nick, Richard, what do we need to do with this bug? It keeps popping up in
> triage.

This is handling an edge of an edge race condition, and will make this client much more robust than other clients.

Feel free to de-prioritize.