1174956 - Refine server version warning

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

We may want to refine the conditions where WebIDE shows the "server is too new warning".

As one example:

(In reply to Nick Alexander :nalexander from comment #14)
> Is this supposed to /warn/ or to /fail/?  I think it warns, but it's
> extremely off-putting for local *browser* developers: almost by definition,
> my local (custom) Fennec build is newer than my Desktop nightly.  Can we
> turn this off when connecting to custom builds?

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

The challenge here is that potentially any server (Fennec, FxOS, etc.) newer than the client (desktop Firefox) can lead to random failures, since the client can't possibly know how to speak to server newer than itself.

The warning was added due to previous issues we've seen where FxOS devs would have a device with trunk / m-c code and connect using Dev. Ed. Firefox.

I am not sure if local / custom build is a helpful flag here.  In the FxOS case, many of the issues came from FxOS devs using their own builds.

Maybe we should just allow a few days of "newness" to be less annoying in this case, while still covering most problem cases?  Or even more relaxed, only check the major Gecko version?

The more we relax the warning, the more useless it becomes, so it's hard to choose the right balance.

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> The challenge here is that potentially any server (Fennec, FxOS, etc.) newer
> than the client (desktop Firefox) can lead to random failures, since the
> client can't possibly know how to speak to server newer than itself.
> 
> The warning was added due to previous issues we've seen where FxOS devs
> would have a device with trunk / m-c code and connect using Dev. Ed. Firefox.
> 
> I am not sure if local / custom build is a helpful flag here.  In the FxOS
> case, many of the issues came from FxOS devs using their own builds.
> 
> Maybe we should just allow a few days of "newness" to be less annoying in
> this case, while still covering most problem cases?  Or even more relaxed,
> only check the major Gecko version?

I think checking major versions is long-term valuable, but it's a much larger commitment from the development team.
 
> The more we relax the warning, the more useless it becomes, so it's hard to
> choose the right balance.

Of course, the more we train developers to ignore the warning, the more frustrating it becomes to actually see version mismatches as causes for errors.

I think a week one way or the other would be reasonable.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Alex, any thoughts?

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I've already watched numerous people get this warning unnecessarily during the work week.  Let's tweak it.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=931e597ba5da

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: 0001-Bug-1174956-Allow-device-to-be-up-to-one-week-newer-.patch

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Nick, can you test a Firefox build from the try run[1] to verify the warning is now gone for your use case?

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=931e597ba5da

Comment 8

[Alexandre Poirot [:ochameau]]

Comment on attachment 8626040 [details] [diff] [review]
0001-Bug-1174956-Allow-device-to-be-up-to-one-week-newer-.patch

Review of attachment 8626040 [details] [diff] [review]:
-----------------------------------------------------------------

I think it will still let some core contributors, building new images on daily basis, hit some regressions.
But this will still catch people using release or nightly with autoupdated phones.

The only safe way I can think about is checking against a build id we could specify in a pref of something on device, that we know when we need a desktop newer than X. But that requires to anticipate and always know when such situation happens and not forget to bump the id...

May be 7days is enough, if core contributors at least start seeing that, they will learn about these possile breakage and may be think about that when seeing regressions.

::: browser/devtools/webide/content/webide.js
@@ +918,5 @@
> +      let deviceDate = this.buildIDToDate(deviceID);
> +      let localDate = this.buildIDToDate(localID);
> +      // Allow device to be too new by up to a week.  This accomidates those
> +      // with local device builds, since their devices will almost always be
> +      // newer than the client.

too new -> newer ?
accomidates -> accomodates ?

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Alexandre Poirot [:ochameau] from comment #8)
> ::: browser/devtools/webide/content/webide.js
> @@ +918,5 @@
> > +      let deviceDate = this.buildIDToDate(deviceID);
> > +      let localDate = this.buildIDToDate(localID);
> > +      // Allow device to be too new by up to a week.  This accomidates those
> > +      // with local device builds, since their devices will almost always be
> > +      // newer than the client.
> 
> too new -> newer ?
> accomidates -> accomodates ?

It also needs an extra "m" too it seems...  Yay, English.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/05fe019cf2ed

Comment 11

[Nick Alexander :nalexander [he/him]]

jryans: sorry, too busy to get to this in a timely manner.  I'll re-open or file a new ticket if this isn't sufficient.  Thanks!

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/05fe019cf2ed