Closed
Bug 1167827
Opened 9 years ago
Closed 9 years ago
Quietly ignore unknown visit codes
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: rnewman, Assigned: rnewman)
Details
Attachments
(1 file)
1.43 KB,
patch
|
mak
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Failing hard when encountering an unknown visit code seems counter-productive. Let's follow Postel's Law and accept unknown visit types in desktop Sync.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8615744 -
Flags: review?(mak77)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Comment 2•9 years ago
|
||
Comment on attachment 8615744 [details] [diff] [review] Quietly ignore unknown visit codes. v1 Review of attachment 8615744 [details] [diff] [review]: ----------------------------------------------------------------- I assume there are no tests for these failures that you should update? ::: services/sync/modules/engines/history.js @@ +276,5 @@ > if (!visit.date || typeof visit.date != "number") { > this._log.warn("Encountered record with invalid visit date: " > + visit.date); > throw "Visit has no date!"; > } should we do the same here? The case doesn't look very different, so it seems sensible to do.
Attachment #8615744 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #2) > I assume there are no tests for these failures that you should update? Huh, there is a test! Missed it the first time. Will update. > should we do the same here? The case doesn't look very different, so it > seems sensible to do. Having no date seems like a legit problem to me, but probably dying when processing the record isn't useful, so I guess I agree!
Assignee | ||
Comment 4•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/080a87125d30
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8615744 [details] [diff] [review] Quietly ignore unknown visit codes. v1 Approval Request Comment [Feature/regressing bug #]: None. [User impact if declined]: Should be none. This tiny patch just makes Sync more fault tolerant. [Describe test coverage new/current, TreeHerder]: Existing tests. [Risks and why]: It's possible for a malformed or extended history record to never be successfully processed by Sync, which will eventually be reported as an error. This patch eliminates most of that for history visits. [String/UUID change made/needed]:
Attachment #8615744 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox40:
--- → affected
status-firefox41:
--- → affected
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/080a87125d30
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 7•9 years ago
|
||
Comment on attachment 8615744 [details] [diff] [review] Quietly ignore unknown visit codes. v1 OK, let's do it.
Attachment #8615744 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 8•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/794f06f32268
Flags: in-testsuite+
Updated•6 years ago
|
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.
Description
•