Refine server version warning

RESOLVED FIXED in Firefox 42

Status

()

Firefox
Developer Tools: WebIDE
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

unspecified
Firefox 42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

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?
(Assignee)

Comment 1

3 years ago
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.
(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.
(Assignee)

Comment 3

3 years ago
Alex, any thoughts?
Flags: needinfo?(poirot.alex)
(Assignee)

Comment 4

3 years ago
I've already watched numerous people get this warning unnecessarily during the work week.  Let's tweak it.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Flags: needinfo?(poirot.alex)
(Assignee)

Comment 6

3 years ago
Created attachment 8626040 [details] [diff] [review]
0001-Bug-1174956-Allow-device-to-be-up-to-one-week-newer-.patch
Attachment #8626040 - Flags: review?(poirot.alex)
(Assignee)

Comment 7

3 years ago
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
Flags: needinfo?(nalexander)
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 ?
Attachment #8626040 - Flags: review?(poirot.alex) → review+
(Assignee)

Comment 9

3 years ago
(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.
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!
Flags: needinfo?(nalexander)
https://hg.mozilla.org/mozilla-central/rev/05fe019cf2ed
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41

Updated

3 years ago
Target Milestone: Firefox 41 → Firefox 42
You need to log in before you can comment on or make changes to this bug.