Open Bug 1494424 Opened 6 years ago Updated 2 years ago

Should we stop throwing in queueResponse server/main.js if transport was destroyed

Categories

(DevTools :: Framework, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: jdescottes, Unassigned)

References

Details

This might be best to discuss in a RFC but logging this as a follow up to Bug 1493968.

We had a "race condition" in an about:debugging test that made it so that we could be asking for the list of tabs right before closing the connection. Even though we can and will fix it to make the behavior more predictable, I am not sure throwing is really justified in this case:

https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/devtools/server/main.js#1589-1592

The client may be destroyed by a user action and it is not trying to cancel requests on the server when doing so. So it simply makes sense that some responses cannot reach the client back if this client was destroyed and there is nothing the server can normally do about that.

Logging an error rather than having an unhandled promise failure would seem reasonable.
I'm wondering if this exceptions helps chasing leaks reports?
If the actors are still pending at the test end, they may hold references to the content document and end up reporting document leaks on debug builds.

I'm not totally sure about that, but bug 1442312 was the bug where we introduced these exceptions and it helped debugging an about:debugging failures. Pending exception during test end made a test to fail and these exceptions helped knowing why/what pending request lead to a test failure.
Unfortunately, try builds from this bug are outdated and there isn't enough info in the bug to know exactly what was the precise test error: a leak? a failure? Would it be just the exception about this.transport being null that broke the test?
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.