1167827 - Quietly ignore unknown visit codes

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

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.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Quietly ignore unknown visit codes. v1

Comment 2

[Marco Bonardo [:mak]]

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.

Comment 3

[Richard Newman [:rnewman]]

(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!

Comment 4

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/080a87125d30

Comment 5

[Richard Newman [:rnewman]]

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]:

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/080a87125d30

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8615744 [details] [diff] [review]
Quietly ignore unknown visit codes. v1

OK, let's do it.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/794f06f32268