Closed
Bug 1235370
Opened 9 years ago
Closed 9 years ago
Implement ProtocolError
Categories
(DevTools :: Debugger, defect)
DevTools
Debugger
Tracking
(firefox46 affected)
RESOLVED
DUPLICATE
of bug 1256397
Tracking | Status | |
---|---|---|
firefox46 | --- | affected |
People
(Reporter: linclark, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Splitting this out from Bug #1037992
This is the latest review from Bug 1037992, comment 15, by jryans:
Review of attachment 8664978 [details] [diff] [review] [diff] [review]:
-----------------------------------------------------------------
::: devtools/server/protocol.js
@@ +907,5 @@
> console.error(err);
> if (err.stack) {
> dump(err.stack);
> }
> + if (err instanceof ProtocolError) {
Please add a test of this new protocol.js behavior. See devtools/server/tests/unit/test_protocol_*.
@@ +1438,5 @@
> + * Optional. Other fields to be included in the error packet.
> + * @constructor
> + */
> +var ProtocolError = Class({
> + initialize: function (error, message = "", packet = {}) {
Hmm, any reason to prefer this style over just accepting an object for the whole thing including error and message?
Does it make sense to extend from the Error object?
Reporter | ||
Comment 1•9 years ago
|
||
Notes from IRC discussion with Eddy:
ProtocolError was introduced so that additional properties in an error could be communicated to the client. For example, the lastPausedUrl property [1] is used by some client code [2].
With the current ProtocolError patch, the additional properties won't reach the client code. This is because protocol.js only uses packet.error and package.message [3].
Rather than rewriting how protocol.js handles errors (or putting in some other work around) the current plan is to figure out where the client code depends on these custom properties. If possible, we will rewrite the client code to not depend on these custom properties. Then we could use regular Errors and wouldn't need to rewrite protocol.js to handle custom error packets.
[1] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/script.js?from=script.js%3A1230#1002
[2] https://dxr.mozilla.org/mozilla-central/source/devtools/client/debugger/debugger-controller.js#378
[3] https://dxr.mozilla.org/mozilla-central/source/devtools/server/protocol.js?from=protocol.js%3A1230#1230
Assignee: nobody → lclark
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•9 years ago
|
||
I am going to unlick this cookie.
Assignee: lclark → nobody
Status: ASSIGNED → NEW
Comment 3•9 years ago
|
||
Going to fix this a part of bug 1256397
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•