Component Manager Error Style Guide

Motivation

Above all errors should be useful. That is their only purpose for existence, otherwise we would silently fail and leave everyone to wonder what happened.

Errors should provide the information their users need to take appropriate action. Sometimes that user will be a match in code, sometimes it will be a human looking at a log buffer. Keeping the user in mind is key to making something useful for them!

Simply put, if you follow the principles below:

• Errors will become more meaningful
• Errors will become easier to handle
• Component manager's codebase will become easier to read
• More bugs will be avoided
• Bugs which trigger errors will be easier to fix

Principles

The principles below are only guidelines, albeit strong ones, about how to make meaningful and useful errors. If you think there is a scenario where these guidelines are wrong, use your best judgment, oh, and update this doc.

DO NOT use panic!, assert!, unwrap(...) and expect(...) carelessly

Component manager is not supposed to crash. If component manager crashes, the system will reboot and user data can be lost.

There are very few situations where crashing is acceptable:

• Asserting on internal invariants may be appropriate, if violating the invariant breaks fundamental assumptions about component manager's behavior
  • This is potentially unsafe and can expose user data or allow an attacker to persist their exploit.
  • Examples: Component state
    • We assert that a component whose child is being destroyed cannot be in a New or Unresolved state.
    • We assert that a URL scheme was not registered to multiple component resolvers.
  • Examples: Argument validation
    • We assert that a static child cannot be given numbered handles or dynamic offers.
  • Examples: Expectations from the rest of the system
    • We assert that capabilities like the Hypervisor have the correct zircon properties on their handles.
    • We assert that creating a Zircon DebugLog object should not fail.
  • Examples: Component manager configuration
    • We assert that the BootFs filesystem can be initialized.
    • We assert that the runtime configuration of component manager was successfully loaded from BootFs.
• fidl::endpoints::create_* methods are safe to unwrap. They create zircon channels and wait for packets on those created channels.
• Asserting on already-verified invariants is acceptable if it results in cleaner code.
  • Example: Checking that map.contains_key(key) is true and then calling map.get(key).unwrap() or map[key].
  • When possible, prefer to rewrite the code using if let or match so that the unwrap/panic is not necessary.

DO NOT use println!

While println! integrates with debuglog in component manager, the tracing log library offers more features and is used more widely across our codebase. The tracing library also integrates with debuglog, has convenient macros and supports structured logging.

This is currently guarded by a lint rule.

DO NOT use the anyhow library

The anyhow library makes it impossible to build well-structured error types and match on them.

There are no known exceptions to this rule.

DO use the thiserror library to create custom error types

The thiserror library automates the std::error::Error implementation for custom error types. Avoid implementing std::error::Error by hand.

There are no known exceptions to this rule.

DO create a custom error type for a specific feature/action in Component Manager

Custom error types help enumerate all possible error states for your feature/action. They will not be tied to any other area of component manager and can be maintained independently of the rest of the codebase.

DO consider whether implementing Into<ModelError> for your custom error type is necessary

ModelError has many enum variants for different error categories. It may not be necessary to add your custom error type to ModelError at all.

DO NOT add precise errors to ModelError

Prefer creating a custom error type to represent all errors for your feature/action, even if you currently have exactly one error. If a precise error is added directly to ModelError, developers can lose context about where the error is applicable.

DO NOT store generic error types like CloneableError

Generic error types make it impossible to build well-structured errors. There is no idiomatic way to match on these generic types.

The only exception to this rule is if the error comes from an external library that is using a generic error type. Changing the external library might not be possible, so storing the generic is acceptable.

DO NOT use existing error types for behavior that is “close enough”

When root-causing bugs in component manager, errors that incorrectly describe the state of the world can make things difficult to debug, especially in component manager. Prefer creating an error type to precisely describe your distinct error state. Another option is to add an enum to the existing error type that provides additional classification.

There are no known exceptions to this rule.

DO consider whether logging is absolutely necessary

One way to think of component manager is as an uber library of sorts. As a library it is often true that component manager can not know if the error is meaningful. Consider, for example, what would happen if a UDP library logged every dropped packet.

While there is no universally accepted advice on this matter, there are some tips that should be followed for component manager:

• Be conservative with logging.
• Logs that were created to assist with development/debugging must be removed or set to DEBUG log level before submitting.
• Avoid adding logs in hot code paths. This can produce log spam and spam is not useful.
• If an error is also being returned to a client over FIDL with the same amount of detail, do not log the error.
• Conversely, if some amount of detail is being lost when sending errors over FIDL, it is acceptable to log the detailed error message.
  • Also consider why the client isn't getting the detailed error.
• Log errors where there is no client involved and the error is likely to be important in debugging an issue.
• Respect the logging guidelines set in RFC-0003

DO add component identifiers to logs and errors

Component manager often produces errors that are related to a specific component. Ensure that errors include component identifiers for easy debugging. For logs, use the component-scoped logger or include the component identifier in the message.

Acceptable component identifiers (in order of preference): monikers, component URLs, instance IDs

DO NOT print Debug strings of objects in a log message

The Debug trait and the corresponding {:?} formatting specifier should only be used for interactive debugging purposes. Outputting the equivalent of a JSON object into the logs makes them harder to understand. Prefer printing human-readable error messages to logs.

There are no known exceptions to this rule.

​DO NOT store a stringified error message

Do not store the Debug or Display string of an error. String errors have no reliable structure and are impossible to match on. Always prefer to store the error as-is.

Some errors in external libraries do not implement required traits like Clone or PartialEq. In those rare situations, if it is impossible to add those traits to the external library, it is acceptable to stringify the error message and store that instead.

DO NOT create functions for each error variant

Creation functions for error variants are unnecessary. They allow implicit conversions for some types and they also hide field names. Prefer to create the error type directly and set field names manually.

There are no known exceptions to this rule.

DO think like a crate author

Component manager is a large crate. We already moved routing into its own crate, we might do more in the future. Consider what error types you'd use if the particular section of code you're working on were part of its own crate. What would reasonable error types be for the logical collection of code in the crate?