Skip to content
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

Improve or delete ConfigurationError when creating new Oak Nodes #1027

Closed
tiziano88 opened this issue May 26, 2020 · 3 comments
Closed

Improve or delete ConfigurationError when creating new Oak Nodes #1027

tiziano88 opened this issue May 26, 2020 · 3 comments

Comments

@tiziano88
Copy link
Collaborator

When creating a new Node or Pseudo-Node in the Oak Runtime, we represent errors with values of this enum:

/// An enumeration for errors occuring when building [`Configuration`] from protobuf types.
#[derive(Debug)]
pub enum ConfigurationError {
AddressParsingError(AddrParseError),
IncorrectPort,
NoHostElement,
CertificateParsingError,
WasmiModuleInializationError(wasmi::Error),
}

It only contains some limited amount of details about the failure, and it really needs to be complemented by logs anyways, so we should probably either augment it with all the information needed to identify the failure, or get rid of it and just rely on logging at node creation time.

@tiziano88
Copy link
Collaborator Author

Note that currently we just return ErrInvalidArgs to the calling node if node creation fails:

// This only creates a Node instance, but does not start it.
let instance = node::create_node(
&self.application_configuration,
config,
&self.grpc_configuration,
)
.map_err(|err| {
warn!("could not create node: {:?}", err);
OakStatus::ErrInvalidArgs
})?;

If oak_loader is built with logging enabled, then the Runtime will log details about the failure, but otherwise they will be lost forever, which is not ideal.

Perhaps we should have a mechanism for returning a string error message alongside an error message from ABI function calls, so that the error message may be printed by the calling node using regular logging pseudo-node (if one available and IFC is satisfied).

@daviddrysdale (or anyone else): is there a good pattern that may be used here, short of just adding additional parameters to each ABI function calls with pointer to a buffer in which to write an error message?

@tiziano88
Copy link
Collaborator Author

In this particular case, an alternative pattern could be to implement node_create as an RPC between the node and the runtime, rather than as an ABI call; we could define a gRPC-like interface for the Runtime, in which one of the methods would allow passing in all the parameters (including node configuration and label, which are conveniently already expressed as protobuf anyways), and get back a structured response (again as protobuf) or an error. This would presumably also allow to move / add more functionality to the Oak Runtime in a more type-safe and self-documenting way.

For instance, it would allow us to move random_get from a dedicated ABI function call, to a RPC to the Runtime itself.

There are pros and cons, but I think it would be interesting to consider this approach.

@ipetr0v
Copy link
Contributor

ipetr0v commented Jan 13, 2021

Do we need ConfigurationError for some complex logic, or it can just be replaced by anyhow::Error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants