Exceptions Overhaul

[1st1]

Abstract

We propose a series of changes to the current implementation of exceptions in EdgeDB. The goals include:

• Specifying a binary encoding of exceptions to include additional context information about errors. The current encoding was inspired by PostgreSQL protocol and is currently undocumented.

• Adding a way to raise exceptions from EdgeQL.

• Exposing builtin exception types and making them introspectable.

• Making it possible for EdgeDB users to define their own exceptions.

This discussion post will later be converted into an RFC.

Proposal

Error types

We propose to add a new built-in module called errors and a new concept of "error types":

module errors {

  error type Error {
    # Exception message
    required property message -> str;

    # Location of the error
    property location -> tuple<
      uri: str,
      range: tuple<
        start: tuple<line: int64, character: int64>,
        end: tuple<line: int64, character: int64>
      >
    >;

    # Additional information, such as hints and details
    multi property related_info -> tuple<
      message: str,
      uri: str,
      range: tuple<
        start: tuple<line: int64, character: int64>,
        end: tuple<line: int64, character: int64>
      >
    >;
  }

  error type InternalServerError extending Error;

  error type UnsupportedFeatureError extending Error;

  error type ProtocolError extending Error;
  error type BinaryProtocolError extending ProtocolError;

  # ... all other types from
  # https://github.com/edgedb/edgedb/blob/master/edb/api/errors.txt

}

The above structure is inspired by the LSP. The idea is that the internal EdgeDB APIs once aligned with the proposed structure would enable future support of LSP right in EdgeDB itself.

The error types defined in the errors module, are conceptually similar to regular types, but have an additional restriction: it will not be allowed to INSERT, UPDATE, DELETE, or SELECT error types. They will only be directly usable via the new throw clause.

The types will allow subclassing to allow users to define their own error kinds:

module foo {
  error type MyError extending errors::Error {
    property metadata -> json;
  }
}

Since the exception types are quite different from "normal" EdgeDB object types, we propose to reflect via a separate new schema::ErrorType type;

The "throw" clause

We propose to add a new "throw" clause to EdgeQL allowing users to raise exceptions; example:

SELECT THROW errors::RuntimeUserError { message := 'something went wrong' };

# or equivalent short version (always raises `RuntimeUserError`):

SELECT THROW 'something went wrong';

The clause would return a new bottom-type called never. It is a subtype of, and assignable to, every type; however, no type is a subtype of, or assignable to, never.

This allows users to implement runtime-assertions, as in:

WITH
    MODULE schema
SELECT Type {
    id,
    name,
    kind := 'object' IF Type IS ObjectType ELSE
            'scalar' IF Type IS ScalarType ELSE
            'array' IF Type IS Array ELSE
            'tuple' IF Type IS Tuple ELSE
            THROW 'unknown type kind',
    [IS ScalarType].enum_values,
    [IS ObjectType].union_of,
    # ...
}

Implementation

• Continue using PostgreSQL exceptions as the base mechanism.

• In binary, exceptions will be encoded the same way as regular data messages. The ErrorResponse protocol message will include both a type descriptor and an encoded specific error.

• For exceptions constructed in runtime we will construct the type descriptor during compilation, and the actual error data in runtime (via the SELECT record_send(row('myerror', 'foo')); kind of SQL query).

• Serialized exceptions will include implicit __tname__ (fully qualified error type name) and __tid__ (error type id) properties.

Backwards Compatibility

This will be a backwards-incompatible change on the protocol level. The high-level client code and EdgeQL queries will continue working as before.

[1st1]

Related and soon to become obsolete PR: #1829

[tailhook]

I like most of the proposal except:

type MyError extending errors::Error {

I think error types more similar to scalar types than to object types so either we should have another qualifier:

error type MyError extending errors::Error {
}

Or we drop SCALAR qualifier and decide exclusively by the type being extended:

type Color extending enum<Red, Blue, Green> {}

[1st1]

I think error types more similar to scalar types than to object types so either we should have another qualifier:

I kinda like your error type idea, but I'm not sure that error types are closer to scalar types. I can certainly see a possibility when an error type links to some object types. And some subtypes of error::Error will have additional properties, like ConstraintViolationError will have additional context of what value violated which property/link constraint.

[1st1]

@elprans, I'm curious what you think about error type. I like it because error types will have limitations (like you cannot INSERT them). Having an explicit qualifier rather than relying on presence of errors::Error in the ancestors list is IMO a cleaner approach. And I already proposed to have a separate schema::ErrorType reflection type for them, so I think this all ties nicely together.

[elprans]

Yes, error type Foo is the cleanest approach, as errors are a completely distinct schema object hierarchy.

[1st1]

I've updated the proposal to feature "error types".

[tailhook]

Also note that character in the LSP is UTF-16-based, not actual unicode character which is only friendly for JavaScript (is this easy enough to compute in python?). So we should be explicit about whether we adhere to this behavior or not.

[1st1]

Also note that character in the LSP is UTF-16-based, not actual unicode character which is only friendly for JavaScript

That's an interesting point. The offsets should be tailored for the same encoding we received the query in, and that's usually UTF8 for us (single character). So it looks like when we implement the LSP, we'll need to make sure that the internal APIs, e.g.

raise EdgeQLSyntaxError(..., location=...)

are behaving properly and that we serialize ranges with a correct encoding in mind.

[tailhook]

Additionally to encoding type-id and type-name into the message I think we need to encode all chain of superclasses in the error message. This is needed for the following (here is Rust, but I'm pretty sure other languages will need this too):

1. We define (potentially subset of) exceptions as types
2. Then we have error.is::<IntegrityError>(). This should work even if specific error is an unknown subclass of the error

Actually we need two things:

1. Stable (i.e. hardcoded) ids for all builtin error types (which was probably assumed, but I don't see it anywhere)
2. Send error superclasses chain within the error message

Parent chain probably should also contain type names because I'd like declaring errors like this:

#[derive(EdgedbError)]
#[edgedb(module="default")]
struct EdgedbError;

And this should work without extra introspection queries. So we should either make error ids derived from their qualified names or make superclass names available within the error message itself.

[1st1]

We've recently discussed if we want to support multiple inheritance of error types. Few notes:

• We want to limit error types to support single inheritance. That will make it easier to reflect them to languages that don't support MI, and that's the majority of them.

• We'll use our annotations mechanism to "tag" some exceptions. E.g. all exception that are safe for retrying the query could have a "retry-safe" annotation. And in our bindings we could support something like: catch (e) { if (e instanceof EdgeDBError && e.hasTag('retry-safe')) { ... }}

[tailhook]

Also take a look at this trait: https://docs.rs/miette/3.2.0/miette/trait.Diagnostic.html
It's the state of the art of Rust errors.

There are subtle differences in how miette::Diagnostic vs the structure above.

[tailhook]

Is this a good idea that location info is in the very base error? In most errors, like InternalServerError and ClientInterface error those properties are absent. It only makes sense for subclasses of QueryError I think (even in some QueryErrors like SessionTimeoutError there is no location). Is there a reason to attach it to the top most one?