1394073 - Uploader doesn't declare store failure if "last payload" fails to upload

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, enhancement, P1)

Description

[:Grisha Kruglov]

Current behaviour if the "last payload" - in most syncs that is also the first and payload - fails to upload is to declare success of a store operation, while also enumerating failed records via onRecordStoreFailed delegate callback.

This seems like a strange remnant of the non-batching uploader, or perhaps just a mistake.

Thankfully it's harmless, thanks to behaviour upsream: we consider any remote record store failures as a "sync fail" on the SynchronizerSession level, and so we don't bump our timestamps for the remote repository.

Comment 1

[:Grisha Kruglov]

Attachment: Bug 1394073 - Call onStoreFailed in case last payload fails in the uploader

Review commit: https://reviewboard.mozilla.org/r/172894/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/172894/

Comment 2

[:Grisha Kruglov]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4e582436d5743bbf2ffd5e8207269b8cccff98b

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 8901407 [details]
Bug 1394073 - Call onStoreFailed in case last payload fails in the uploader

https://reviewboard.mozilla.org/r/172894/#review178684

This makes me slightly uneasy, but I can't find any reason in the code for being uneasy, so roll on.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4ce8ee833e9
Call onStoreFailed in case last payload fails in the uploader r=rnewman

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f4ce8ee833e9