cdp
cdp copied to clipboard
Distinguishing between errors
Is it currently possible to distinguish between response errors and network errors in RPCC?
For example, with the DOM.getNodeForLocation method, it can sometimes return an error: No node found at given location. I don't really want to error out in all cases of this error, but for things like the RPCC websocket disconnecting, I would like to error out immediately.
Is there anyway to distinguish between these types of errors? Here's some (incorrect) psuedocode of what I'm trying to accomplish:
var args cdpcmd.DOMGetNodeForLocationArgs
if e := json.Unmarshal(req.Params, &args); e != nil {
return raw, e
}
reply, err := c.DOM.GetNodeForLocation(ctx, &args)
if err != nil {
if err.(*ResponseError) {
// handle differently
}
return raw, err
}
return json.Marshal(reply)
Thanks!
I haven't really found the need to differentiate between RPC errors and errors on the socket, as such I haven't put enough thought into how to deal with these kinds of situations.
One problem here is that relying on the RPC error tightly couples your code with the current behavior of the CDP protocol. What would you test for if you knew the Code/Message/Data of the RPC error? The error code returned by this function is -32000 with stands for a general Server error (according to the JSON-RPC 2.0 spec). This could mean anything. Also, the message (in this case No node found at given location) is only defined in the chromium/WebKit source and could easily have changed in a future release.
If we assume that the above is not an issue (e.g. we only want to differentiate RPC and socket errors) then technically this should be possible by doing something along the lines of (untested):
if rpcErr := cdp.ErrorCause(err); rpcErr != nil {
if rpcErr, ok := rpcErr.(*rpcc.ResponseError); ok {
// rpcErr.Code ?
}
}
But I'm not very satisfied with this API. I'm thinking I would like to unexport ResponseError since it's very much an implementation detail. I'd much rather expose a different kind of API for this, maybe something like rpcc.StatusFromError(err). In turn, rpcc.Status could have the following methods: Code(), Message(), Data() (methods can allow for some decoupling via interfaces).
Since cdp errors implement interface causer { Cause() error }, we could test for this in StatusFromError so that we could simply do: if status := rpcc.StatusFromError(err); status != nil { /* do something with e.g. status.Code() */ }.
I haven't put a lot of thought into the naming above, I blatantly stole it from grpc-go/status/status.go.
Another option (to Status above) is exposing some new methods on rpcc, e.g. rpcc.ErrorCode(err), rpcc.ErrorMessage(err) and rpcc.ErrorData(err), although the last one's usefulness is questionable.
Thoughts / suggestions are welcome 😄.
I just confirmed that the above works, more accurately, it might look like this:
_, err = c.DOM.GetNodeForLocation(ctx, dom.NewGetNodeForLocationArgs(x, y))
if err := cdp.ErrorCause(err); err != nil {
if err, ok := err.(*rpcc.ResponseError); ok {
log.Printf("Got error code: %d", err.Code)
} else {
return err
}
}
Or a slightly longer version if you care about the original error:
if err != nil {
if err2 := cdp.ErrorCause(err); err != err2 {
if err2, ok := err2.(*rpcc.ResponseError); ok {
log.Printf("Got error code: %d", err2.Code)
} else {
return err
}
} else {
return err
}
}
I'm still thinking about nicer solutions for this though 😉.
Apologies for the spamming, I'm going to indulge my paranoia a bit 😏.
I couldn't remember how the CDP protocol deals with miscellaneous RPC errors, so I did the following test:
var args = struct {
X string `json:"x"`
Y int `json:"y,omitempty"`
Z int `json:"z"`
}{
X: "1000",
Z: 1000,
}
err = rpcc.Invoke(ctx, "DOM.getNodeForLocation", &args, nil, conn)
fmt.Println(err)
Result:
rpc error: Invalid parameters (code = -32602, data = x: integer value expected; y: integer value expected)
So it does try to use predefined RPC errors where applicable (-32602 here). This makes it perhaps a bit more safe to rely on the RPC error code (e.g. 32000 returned when no node is found). Although, chromium seems to use this error code as a catch-all whenever there isn't a predefined RPC error. To really trust that we're not letting unknown errors pass by due to these kinds of checks, the only way I can think of is to actually test both error code and the message string (No node found at given location).
I worry that these errors could easily vary between browsers (we could be using Chrome, Safari or Edge, for example) or browser versions. Future versions of the protocol might introduce new errors for these functions that we might want to handle, etc.
Some food for thought.
@mafredri I apologize for the late response here and I really appreciate your hard work on this issue!
Sidenote: I noticed you use typescript on a few of your other open source projects, so I think you might be interested in browser client that I've been busy finishing up. Not quite ready yet, but I just invited you to the repo: https://github.com/matthewmueller/chromie. I modeled a lot of it after CDP's API design :-)
Back to the matter at hand! Yah I definitely agree that it would get hairy trying to make sense of the error codes and messages. The protocol changes so much. The motivation for this issue is that if the client hasn't fully loaded the page yet and you call DOM.getNodeForLocation on part of the blank page, you'll get an RPCC error.
I'd basically like to know which type of error I'm dealing with, not really the exact error:
- Is this an error with writing or reading from the socket?
- Is this a
ResponseErrorfromrpcc.Response. E.g. https://github.com/mafredri/cdp/blob/bcfae006805576509c9808cf0638e09cb1441ed2/rpcc/conn.go#L250-L260
If it's an error from reading or writing to the socket, you'd probably want to kill the program, if it's a user error, like trying to get a node before the page loads, I'd like to treat that differently, like show a warning
Is that possible?
Also in other news, I had no idea the chrome devtools protocol was coming to other platforms! that's awesome :-D
I apologize for the late response here and I really appreciate your hard work on this issue!
No worries, I'm still thinking about how to solve this to provide the best ergonomics!
Back to the matter at hand! Yah I definitely agree that it would get hairy trying to make sense of the error codes and messages. The protocol changes so much. The motivation for this issue is that if the client hasn't fully loaded the page yet and you call
DOM.getNodeForLocationon part of the blank page, you'll get an RPCC error.
Yeah, this seems like a solid use-case that should be supported (officially)!
Is that possible?
You can totally do it right now, check https://github.com/mafredri/cdp/issues/14#issuecomment-314476898. But as I said, I'm still thinking of a better way to do this, so just a heads up that I might break it in the future 😄.
So to sum up: the *rpcc.ResponseError is what you're looking for. If it's not one of those, then it is (at least currently) either a rpcc.ErrConnClosing or an error that originates from gorilla/websocket or net or wherever.
The rpcc package doesn't wrap errors, only cdp (to ease with debugging) so you have the option to test for specific errors after checking the causer.
Sidenote: I noticed you use typescript on a few of your other open source projects, so I think you might be interested in browser client that I've been busy finishing up. Not quite ready yet, but I just invited you to the repo: matthewmueller/chromie. I modeled a lot of it after CDP's API design :-)
Oh, that's really cool, thanks for the invite! The API does look familiar, glad to see it has inspired you 😊. I hope I'll get around to contributing to it a well 🙂.
Also in other news, I had no idea the chrome devtools protocol was coming to other platforms! that's awesome :-D
Not exactly Chrome DevTools, but the protocol is (at least partially) implemented in other browsers as well: RemoteDebug Protocol Compatibility Tables. E.g. for Edge there's the edge-diagnostics-adapter (which is looking a bit stale atm). Beauty of CDP becomes most apparent when all browsers implement it (🤞).
Following along with the Go 2 error proposal(s) to see what improvements we could do here. For one, an improvement that's immediately possible via the proposal (or right now the xerrors package):
var respErr *rpcc.ResponseError
reply, err := c.DOM.GetNodeForLocation(ctx, &args)
if err != nil {
if xerrors.As(err, &respErr) {
if respErr.Code == 30000 {
// handle differently
}
}
return raw, err
}
Which is pretty cool 😄.