1421316 - More versatile error handling in server.TCPConnection

Status: NEW

(Remote Protocol :: Marionette, enhancement, P5)

Description

[Andreas Tolfsen ❲:ato❳]

When server.TCPConnection#onPacket receives a new message and it
is not an array we use error.report to log it to stdout.  We could
possibly construct an error response and send that back in this
case.

I don’t think there is anything we can do if sendResponse or sendError
fails, however.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The code Andreas is referring to here can be found at:
https://dxr.mozilla.org/mozilla-central/rev/c2248f85346939d3e0b01f57276c440ccb2d16a1/testing/marionette/server.js#481-486

Andreas, which kind of error class should be used in this case? I would assume `UnknownError`.

Would you mind to mentor this bug?

Comment 2

[Andreas Tolfsen ❲:ato❳]

I had a another look into this and I can’t find a good way to know
how to construct an error response when Array.isArray(data) fails.
The response packet is meant to be a four-element array such as
[<type>, <id>, <error>, <data>] but if we fail to unmarshal the
command request we won’t know what command ID to respond to.

Do you have any great ideas?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh, indeed. This is really sad. But maybe we could at least use logging in `error.report()`, and get this out as an error, or at least warning?

Comment 4

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #3)

> Oh, indeed. This is really sad.

I guess we could potentially introduce a message ID -1 meaning
something catastrophic went wrong.  That involves building support
for it into clients as well.

> But maybe we could at least use logging in `error.report()`, and
> get this out as an error, or at least warning?

I believe that already happens.