-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Distinguishing between errors #14
Comments
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 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 Since 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 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:
So it does try to use predefined RPC errors where applicable ( 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 I'd basically like to know which type of error I'm dealing with, not really the exact error:
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 |
No worries, I'm still thinking about how to solve this to provide the best ergonomics!
Yeah, this seems like a solid use-case that should be supported (officially)!
You can totally do it right now, check #14 (comment). 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 The
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 🙂.
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 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 😄. |
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:
Thanks!
The text was updated successfully, but these errors were encountered: