feat(supervisor/core): Implement l2 controller #1866
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| debug!(target: "managed_node", "Executing RPC call"); | ||
|
|
||
| let result = f(client).await.map_err(|err| { | ||
| error!(target: "managed_node", "RPC call failed: {:?}", err); |
There was a problem hiding this comment.
| error!(target: "managed_node", "RPC call failed: {:?}", err); | |
| error!(target: "managed_node", ?err, "RPC call failed"); |
?err prints to stdout as "err=<debug-formatted-error>". %err prints to stdout as "err=<display-formatted-error>". usually we want the display formatting unless you have a specific reason to want to debug format it?
The % sigil operates similarly, but indicates that the value should be recorded using its fmt::Display implementation:
There was a problem hiding this comment.
@emhane I'm confused between your code suggestion and comment. So it should be ?err, i.e. Debug formatted?
There was a problem hiding this comment.
better use display format, ie % operator.
since you originally used debug format, ie ? operator, i thought perhaps you had a particular reason for it. for example errors wrapping an enr::Enr are useless in display format because it just shows the base64 serialised enr, while we are mostly interested in reading the enr fields as human readable data for debugging, which we get with debug formatting for the enr::Enr type - but that particular type is an exception.
The 8000 re was a problem hiding this comment.
Wouldn't that be different from how they are used in all other parts of the code? E.g. in node, they have used ?err.
I don't mind changing it to %err, just confirming if that's fine to defer from the followed pattern.
There was a problem hiding this comment.
I think we should use %err in majority of the cases and use?err in specific places where we need debug information.
| if ws_client_guard.is_none() { | ||
| let headers = self.create_auth_headers().map_err(|err| { | ||
| error!(target: "managed_node", %err, "Failed to create auth headers"); | ||
| ClientError::Custom(format!("failed to create auth headers: {}", err)) |
There was a problem hiding this comment.
| ClientError::Custom(format!("failed to create auth headers: {}", err)) | |
| ClientError::Custom(format!("failed to create auth headers: {ett}")) |
nitpick, this syntax is preferred, pls check if there are other places in this pr diff where this can be fixed too
Closes #1680.
ManagedNodeApicall_rpc()toManagedNodeto for calling the API by passing theManagedNodeApifunction along with params.