Closed Bug 1255302 Opened 6 years ago Closed 6 years ago

Notify the server if there is an error handling a webchannel message.

Categories

(Firefox :: Firefox Accounts, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: markh, Assigned: lina)

References

Details

Attachments

(1 file)

As FxA in Firefox becomes more dependent on webchannel messages, we should try and gather data when Firefox sees an error while processing a message - we currently have no visibility on when things go wrong processing these messages. I think the best thing would be to send a webchannel response back to the server with details of the error, and have the server log this in a way that can be queried (eg, via the work Danny is doing with redshift etc)

Vlad and I had a quick chat in IRC. There's currently only 1 message that causes a response to be sent (https://dxr.mozilla.org/mozilla-central/rev/af7c0cb0798f5425d5d344cbaf0ac0ecb1a72a86/services/fxaccounts/FxAccountsWebChannel.jsm#163) and Vlad and I came up with using the same basic mechanism:

* Send a response message with the same command and command-id as the originating message.
* Instead of |data: { ok: canLinkAccount }| we'd use |data: { error: details }| where details could be, say, |{message: exception_message, stack: stack_trace}| - this becomes the de-facto pattern for responses (ie, same command and message ID, body has either |ok| or |error|)
* In most cases we only send this response when there is an error - there's no need to reply when everything worked (although some messages, such as the one above, obviously expect a response every time)

Vlad suggested the client could implement this before the server grew specific support for handling and logging the message, so IMO the client should do this ASAP and a pull request opened for the server to land no later than 48 hitting beta. We'd probably also need a further bug for getting this data into whatever dashboard Danny is coming up with.

What do you think?
Flags: firefox-backlog+
Sounds good to me; maybe we should log them to sentry as though they were unexpected JS errors?
Mark and I talked about this today. I can wire up FxAccountsWebChannel.jsm now, while we wait for server support.
Assignee: nobody → kcambridge
Status: NEW → ASSIGNED
Comment on attachment 8739213 [details]
MozReview Request: Bug 1255302 - Report FxA WebChannel message handling errors to the sender. r?markh

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45113/diff/1-2/
Attachment #8739213 - Flags: review?(markh) → review+
Comment on attachment 8739213 [details]
MozReview Request: Bug 1255302 - Report FxA WebChannel message handling errors to the sender. r?markh

https://reviewboard.mozilla.org/r/45113/#review41645

LGTM, but let's get Vlad to sign off on it too!
Attachment #8739213 - Flags: feedback?(vlad)
Comment on attachment 8739213 [details]
MozReview Request: Bug 1255302 - Report FxA WebChannel message handling errors to the sender. r?markh

https://reviewboard.mozilla.org/r/45113/#review42509

Awesome!
Attachment #8739213 - Flags: review+
Comment on attachment 8739213 [details]
MozReview Request: Bug 1255302 - Report FxA WebChannel message handling errors to the sender. r?markh

Looked through this in reviewboard. I am confused with the BugZilla UI for this, '+'ing something with this comment, hopefully it works.
Attachment #8739213 - Flags: feedback?(vlad) → feedback+
Vlad, for completeness, could you please add a link to the PR/Issue/whatever-cool-kids-use to handle the server side of this?
Flags: needinfo?(vlad)
https://hg.mozilla.org/mozilla-central/rev/7d08ffc48bac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
See Also: → 1320189
Product: Core → Firefox
Target Milestone: mozilla48 → Firefox 48
You need to log in before you can comment on or make changes to this bug.