Closed
Bug 1367407
Opened 7 years ago
Closed 7 years ago
Intermittent browser_animation_mutations_with_same_names.js | A promise chain failed to handle a rejection: - protocol.js:1212 - Error: Connection closed, pending request to animationplayer48, type getProperties failed
Categories
(DevTools :: Inspector: Animations, defect, P2)
DevTools
Inspector: Animations
Tracking
(firefox55 fixed, firefox56 fixed)
RESOLVED
FIXED
Firefox 56
People
(Reporter: intermittent-bug-filer, Assigned: pbro)
References
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:other])
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
daisuke
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 3•7 years ago
|
||
doing a bunch of retriggers:
https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=linux%20pgo%20e10s%20dt&tochange=e52428cb04a0bed3a3a67fe98ec561c608d459aa&fromchange=c0b9404877089ffd6ba3027f761699fa69d6b02c&selectedJob=102341418
top of the list is dt9, then in the middle/bottom it is dt7.
:gl, this had 42 failures yesterday, that is pretty extreme- hopefully we can find a root cause for this- can you get someone from the animation inspector team to look into this?
Flags: needinfo?(gl)
Whiteboard: [stockwell needswork]
Comment 4•7 years ago
|
||
this is a side effect of bug 1309468.
:daisuke, I see you authored the patches in bug 1309468, can you please look at this intermittent failure? I will probably disable this on Tuesday if the failure rate is still high and there is no pending fix.
Blocks: 1309468
Flags: needinfo?(gl) → needinfo?(dakatsuka)
Comment 5•7 years ago
|
||
Thanks Joel,
I'll take a look.
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 9•7 years ago
|
||
this seemed to have stopped around May 29th/30th, do we know why this was "fixed" ?
Comment 10•7 years ago
|
||
I haven't modify at all.
I sent to try-server with my debug print since I want to find the cause, however we could not reproduce the error at this time.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc8e6fb748d6e1c9bad261d92615807b335c4de6
Flags: needinfo?(dakatsuka)
Comment 11•7 years ago
|
||
ok, lets see if this picks up again in the future
Whiteboard: [stockwell needswork] → [stockwell unknown]
Comment hidden (Intermittent Failures Robot) |
Summary: Intermittent browser_animation_mutations_with_same_names.js | A promise chain failed to handle a rejection: - resource://devtools/shared/protocol.js:1212 - Error: Connection closed, pending request to server1.conn8.child1/animationplayer48, type getPrope → Intermittent browser_animation_mutations_with_same_names.js | A promise chain failed to handle a rejection: - protocol.js:1212 - Error: Connection closed, pending request to animationplayer48, type getProperties failed
Comment 14•7 years ago
|
||
This started up again on June 3, perhaps with bug 1368364, a backout. Did something go wrong there?
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(jmaher)
Whiteboard: [stockwell unknown] → [stockwell needswork]
Comment hidden (Intermittent Failures Robot) |
Comment 16•7 years ago
|
||
The devtools tests have race conditions on shutdown, and bug 1242505 provided a new mechanism to whitelist them.
After the backout, this can still be whitelisted using the previously existing thisTestLeaksUncaughtRejectionsAndShouldBeFixed function.
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(jmaher)
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 18•7 years ago
|
||
Hi Daisuke. Do you have some time to possibly look into this intermittent? It picked up again. Let me know if not and I'll make sure to have someone look into this.
Flags: needinfo?(dakatsuka)
Priority: P3 → P2
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 21•7 years ago
|
||
Hi Patrick,
I am investigating this one.
About the phenomenon in case of the bug occur:
1. Open inspector
2. Updated animation ui.
3. Call getTracks() in animation-timeline ( also getProperties() of animation in the function )
4. Called getProperties() of animation actor in server.
5. Re-updated animation ui while processing 4?
( in this test case, we use setTimeout() to append animation later. )
Then, the getProperties() of actor in server does not return any result.
So, the connection might be disconnected when animation ui updates?
Comment 22•7 years ago
|
||
This is my test try with debug print.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=53a000c1b36a36581020cc343b2ddca9d7aa1d5c&selectedJob=105727598
Flags: needinfo?(dakatsuka) → needinfo?(pbrosset)
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 26•7 years ago
|
||
The problem seems to me that we now have several independent async operations in the panel. When running a test, we don't know which is going to complete first, and our tests don't wait for all of them to be done.
If a test ends before an async operation that led to calling the server could complete, then that leads to the error we're seeing here.
In this particular case, the call to `getTracks` hasn't completed by the time the test ends.
The logs you added to your try push helped a lot! Thanks Daisuke.
I'm going to try a few things and report here if I have a solution for this test.
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 27•7 years ago
|
||
Here's a try build with a fix I'm attempting: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d466f4cad70e0c9a5b0f2fa3618942397b8bd32e&group_state=expanded&selectedJob=106294216
Comment 28•7 years ago
|
||
Wao Wao!! No fails!! Thank you so much!!
Comment hidden (mozreview-request) |
Assignee | ||
Comment 30•7 years ago
|
||
I'm fixing only this test with this fix here because it's failing so many times it's quite urgent, but I'm fairly sure other animationinspector tests are failing for the exact same reason, so we should move this piece of code to head.js after this is fixed.
Comment 31•7 years ago
|
||
mozreview-review |
Comment on attachment 8876732 [details]
Bug 1367407 - Early return from timeline render if destroy was called;
https://reviewboard.mozilla.org/r/148070/#review152398
Thank you very much, Patrick!
Attachment #8876732 -
Flags: review?(dakatsuka) → review+
Comment 32•7 years ago
|
||
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f8d83753276
Wait for all tracks to be ready in test; r=daisuke
Comment 33•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•7 years ago
|
Assignee: nobody → pbrosset
status-firefox55:
--- → affected
Comment 34•7 years ago
|
||
bugherder uplift |
Comment hidden (Intermittent Failures Robot) |
Comment 36•7 years ago
|
||
Alas, not actually fixed, it's still failing.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 37•7 years ago
|
||
(In reply to Phil Ringnalda (:philor) from comment #36)
> Alas, not actually fixed, it's still failing.
Are you sure? I've just checked some of the failure logs from comment 35, and they all seem to show logs from before my patch.
Comment 38•7 years ago
|
||
I am seeing some errors today on all branches, the original landing on autoland is:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4f8d83753276
and a more recent instance on autoland:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2b2e73c263b0c4a37436a198232fc4f8f34f6175&selectedJob=106553109
Assignee | ||
Comment 39•7 years ago
|
||
Thanks for the URL, I see failures with my patch applied now. This is depressing ... I had triggered many test run on try before landing. I'll take another look.
Assignee | ||
Comment 40•7 years ago
|
||
I've pushed to try with more logs and understood something I had missed the first time around: when there are animation mutations, we re-render the full timeline, but we do this no matter if it's already being rendered or not. We don't wait for async calls to be done then.
Comment hidden (Intermittent Failures Robot) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 43•7 years ago
|
||
Comment on attachment 8876732 [details]
Bug 1367407 - Early return from timeline render if destroy was called;
Mozreview thinks this is a new version of my earlier fix attempt, but it's really a second, different fix. So flagging you for review again on this Daisuke.
Here's a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c350e512d58d4dc8f8c7c7aafe9ca1085f2fb0f5&group_state=expanded
Attachment #8876732 -
Flags: review+ → review?(dakatsuka)
Comment 44•7 years ago
|
||
mozreview-review |
Comment on attachment 8876732 [details]
Bug 1367407 - Early return from timeline render if destroy was called;
https://reviewboard.mozilla.org/r/148070/#review153356
Oh, I see. So the reason is the connection could not handle if the component is destroyed.
And, this patch might resolve bug 1357526 as well!
Thank you very much, Patrich!
Attachment #8876732 -
Flags: review?(dakatsuka) → review+
Comment 45•7 years ago
|
||
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b79157e0ca55
Early return from timeline render if destroy was called; r=daisuke
Comment 46•7 years ago
|
||
Please request Beta approval on this patch at your earliest convenience.
Flags: needinfo?(pbrosset)
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 48•7 years ago
|
||
Comment on attachment 8876732 [details]
Bug 1367407 - Early return from timeline render if destroy was called;
Approval Request Comment
[Feature/Bug causing the regression]: Not a regression, this is an intermittent test due to async code racing, that got recently very frequent.
[User impact if declined]: No user impact, but a lot of intermittent test failures in CI.
[Is this code covered by automated tests?]: It is a test.
[Has the fix been verified in Nightly?]: Pushed to autoland so far only. I pushed to try before that, and restarted the test run many many times, without noticing any failures.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No. It's basically an early return in the initialization of a sub-panel in devtools that happens if devtools has already been closed. So it's only cleaning up after ourselves.
[Why is the change risky/not risky?]: See above.
[String changes made/needed]: None.
Flags: needinfo?(pbrosset)
Attachment #8876732 -
Flags: approval-mozilla-beta?
Comment 49•7 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Comment 50•7 years ago
|
||
Comment on attachment 8876732 [details]
Bug 1367407 - Early return from timeline render if destroy was called;
devtools fix for intermittent failure, beta55+
Attachment #8876732 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 51•7 years ago
|
||
bugherder uplift |
Whiteboard: [stockwell needswork] → [stockwell fixed]
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Flags: qe-verify-
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Whiteboard: [stockwell fixed] → [stockwell fixed:other]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•