Closed Bug 1148252 Opened 5 years ago Closed 5 years ago

Reading-list sync engine should gracefully handle local item already existing on the server


(Firefox Graveyard :: Reading List, defect)

Not set


(firefox38 fixed, firefox39 fixed)

Firefox 39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed


(Reporter: markh, Assigned: markh)


(Blocks 1 open bug)



(1 file)

Currently we fail to handle an attempt to upload a new item when the server already has it.  This scenario could happen due to our bugs, and possibly even due to a network error in the *response* when we first attempt to upload it (ie, request succeeds but we fail to get a response).

Manually testing with this patch, I saw the problematic article get handled correctly (ie, get the correct syncStatus and GUID) and the log indicate:

> 1427421112074   readinglist.sync        INFO    Attempting to upload a new item found the server already had it: {"status":200,"path":"/v1/articles","body":{"archived":false,"marked_read_on":null,"excerpt":"A Marseille prosecutor says the co-pilot of the Germanwings flight that perished in the French Alps appears to have crashed the plane deliberately.","resolved_url":"","resolved_title":"Germanwings co-pilot crashed deliberately: prosecutor","title":"Germanwings co-pilot crashed deliberately: prosecutor","url":"","is_article":true,"stored_on":1427416672690,"favorite":true,"word_count":null,"read_position":0,"marked_read_by":null,"added_on":1427416366270,"last_modified":1427416672693,"preview":"","unread":true,"id":"75ea03f0e5d04b48ba0bd54e0ac57c0a","added_by":"%USERNAME%'s Nightly on eden"},"headers":{"Content-Length":"955","Backoff":"None","Content-Type":"application/json; charset=UTF-8","Access-Control-Expose-Headers":"Backoff, Retry-After, Alert"}}
Flags: qe-verify-
Flags: firefox-backlog+
Comment on attachment 8584237 [details] [diff] [review]

Review of attachment 8584237 [details] [diff] [review]:

::: browser/components/readinglist/Sync.jsm
@@ +283,5 @@
> +      // So allow 200 but log a warning.
> +      if (response.status == 200) {
> +"Attempting to upload a new item found the server already had it", response);
> +      }
> +      if (response.status != 201 && response.status != 200) {

else if (response.status != 201) {

Attachment #8584237 - Flags: review+
oops - I guess it's not fixed on 39 yet - it's only on fx-team.
Blocks: 1132074
Iteration: --- → 39.3 - 30 Mar
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.