Model property error handling

[steverice]

No description provided.

[FionaBronwen]

Looks great! Added a couple of questions around @error context modifiers, and a nit on clarifying the nuance of using operation errors with @throws vs just operation errors. Nothing blocking though. Thanks for putting this together!

[FionaBronwen]

Does this mean that all errors with Lifecycle.CREATE, Lifecycle.UPDATE, or Lifecycle.DELETE are included as operation results for all input contexts?

[steverice]

I'm playing a bit fast and loose by suggesting that "input" === Create | Update | Delete.

An operation really has two contexts — one for its parameters and one for its return type. So a more formal definition here would be that the errors will be added as possible return types for any operation that includes the Create, Update, or Delete modifiers in either of its two contexts.

An example (that I can add to the doc):

@error(Lifecycle.CREATE, Lifecycle.UPDATE)
model InvalidEmailError {
  message: string;
}

model User {
  @key id: string;

  @visibility(Lifecycle.Create, Lifecycle.Update, Lifecycle.Read)
  @throws(InvalidEmailError)
  email: string;
}

op getUser(id: string): User | UserNotFound;  // will not include InvalidEmailError, does return email field

op createUser(...User): User; // will include InvalidEmailError, does return email field

op deleteUser(id: string): User; // does not include InvalidEmailError, does not return email field

One thing to note is how the context modifiers for the error need to be different from those on the field that throws it.

Do you think it makes sense to tie the context modifiers to the error model? An alternative might be

model User {
  @key id: string;

  @visibility(Lifecycle.Create, Lifecycle.Update, Lifecycle.Read)
  @throws([InvalidEmailError], [Lifecycle.Create, Lifecycle.Update])
  email: string;
}

I can imagine there could be different fields that return the InvalidEmailError for different contexts. To deal with that we'd either need something like this alternative, or have the user create multiple InvalidEmailError models. Sticking with modifiers on the @error decorator would be in line with the @asData and @propagate decorators for GraphQL, where they are tied to the error type. The logic being that the definition of an error includes something about in what contexts it occurs, and how it should be expressed. This is more prescriptive than descriptive; i.e. this sets an implementation convention for spec-first APIs, but would struggle a bit to describe an existing API that does not follow this pattern.

[FionaBronwen]

okay nice-- This example makes it more clear how the @visibility on the field is what determines the application of the error based on the error's context modifier. I forgot about the lifecycle visibility that comes with @get and @post in the Input Contexts and Output Contexts examples already in the doc.

---

I like adding the context modifier to the error, it's clean and I like the context modifier being added at the definition of the error. I don't love the idea of needing multiple versions of the same error which are used in different contexts.

@error(Lifecycle.CREATE, Lifecycle.UPDATE)
model UnauthorizedError {
  message: string;
}

@error(Lifecycle.DELETE)
model UnauthorizedError_Delete { // :(
  message: string;
}

I think the alternative of linking the error to the context from @throws might be easier to work with. Having the context right next to the error in @throws makes it more clear in which context the error will be applied. In a giant TSP schema it may not be as easy to understand which context an error would be applied when adding a new property with @throws if the context is specified on the error model definition.

I see the pros and cons for both implementations. If it's not critical to @throws and @handles should we consider moving error context modifiers to a separate proposal? 😅

[steverice]

I don't love the idea of needing multiple versions of the same error which are used in different contexts.

I know you weren't holding up your example as canonical or anything, but I don't understand why we wouldn't just add Lifecycle.Delete to the UnauthorizedError in that case.

I did neglect to show how the @throws option has its own ugly side:

model User {
  @key id: string;

  @visibility(Lifecycle.Create, Lifecycle.Update, Lifecycle.Read)
  @throws([InvalidEmailError, EmailAlreadyTakenError], [Lifecycle.Create, Lifecycle.Update])
  @throws([PermissionDeniedError], [Lifecycle.Read])
  @throws([MissingFieldError], [Lifecycle.Create])
  email: string;
}

IMO a TypeSpec author would have a difficult time making this determination for each and every field, and it would quickly become outdated. The most likely downside is that a context in which the error actually is thrown is neglected, and a client gets an unexpected error.

On the other hand, I think the most likely downside of associating context modifiers with the @error decorator is that errors are indicated to be possible in locations where they actually are not. This seems like a much less problematic error, since there's no guarantee that any error that actually can be thrown ever actually will be.

In a giant TSP schema it may not be as easy to understand which context an error would be applied when adding a new property with @throws if the context is specified on the error model definition.

I think this is actually the primary feature of this proposal. Abstracting the error specification so that a developer does not have to understand, on every field, when and how it will be handled is the goal.

If it's not critical to @throws and @handles should we consider moving error context modifiers to a separate proposal?

In earlier versions of this proposal I actually labeled it a "bonus". Even now I've billed this as an "optional enhancement".

I will clarify the intent there: that whether or not the context modifiers are added to @error, the rest of the proposal stands.

---

In conclusion (lol), I think I've convinced myself that adding to the @error decorator is still the right choice — and I think I have some better arguments for it that I can add to the proposal.

[FionaBronwen]

It would be cool to see an example of how an emitter might interpret the context modifiers and also how the context modifiers would interact with @throws and @handles.

[FionaBronwen]

nit: It might be nice to provide an example or some additional context about the differences and similarities between using operation errors + @throws and using solely operation errors and when you might use one method versus the other. Ex:

model User {
  @throws(InvalidURLError)
  @handles(PermissionDeniedError)
  profilePictureUrl: string;
}

@route("/user/{id}")
@get
op getUser(@path id: string): User | GenericError;

vs.

model User {
  @handles(PermissionDeniedError)
  profilePictureUrl: string;
}

@route("/user/{id}")
@get
op getUser(@path id: string): User | GenericError | InvalidURLError;

[steverice]

TBH I'm not sure what the answer is there; I think they are redundant in a TSP sense.

It occurred to me while working on this that @throws is a superset of the return type + @error combination. I'd propose removing the latter, but

1. I don't want to blow this up any further
2. I can't think of why putting an error type as a return value should not be valid

However, In a GraphQL sense they are different because operations can be nested inside other things, so the return type implies that the error will occur at that operation result, whereas @throws allows the error to "bubble up" if it has the @propagate decorator. So for GQL, the return type lets you specify that an error that propagates will not propagate for this specific field (i.e., do not make the field non-nullable just because of that error).

I think for Pinterest use case, we would potentially ban errors in return types and use only @throws to avoid confusion.

I'd like to avoid giving any answers that are like "this only matters in GraphQL!" so if you have any thoughts on that as a generic concept, I'd love to hear them.

[swatkatz]

I don't think that @throws is always a superset of return type + @error. I think that operations can have return type errors that are independent of what is thrown by the properties. In @FionaBronwen's 2nd example the InvalidURLError is an error that the operation business logic expects for some reason independent of the profilePictureUrl. What is true though is that if there is a @throws and it is not handled by any of the parent properties, then the operation will have a return type + @error.

[steverice]

What I'm saying is that

@route("/user/{id}")
@get
op getUser(@path id: string): User | GenericError | InvalidURLError;

is equivalent to

@route("/user/{id}")
@get
@throws(InvalidURLError)
op getUser(@path id: string): User | GenericError;

in a protocol like OpenAPI.

But yes, they are not strictly equivalent because other emitters might interpret them differently.

[a-akshat]

This is so cool!

[a-akshat]

This looks amazing!
Very clear and easy to understand.

[swatkatz]

I don't think that @throws is always a superset of return type + @error. I think that operations can have return type errors that are independent of what is thrown by the properties. In @FionaBronwen's 2nd example the InvalidURLError is an error that the operation business logic expects for some reason independent of the profilePictureUrl. What is true though is that if there is a @throws and it is not handled by any of the parent properties, then the operation will have a return type + @error.

[swatkatz]

A question that can be asked is why wouldn't we "document" the error associated with a particular model property using unions i.e.

model Foo {
     @throws(BarNotFoundError)
     bar: Bar
}

@error
model BarNotFoundError {
     code: int32;
     message: string;
}

vs

model Foo {
     bar: Bar | BarNotFoundError
}

I suppose the answer is that with the latter, we are not explicitly "throwing" the error, rather assuming that bar will handle both Bar and BarNotFoundError. This is also how the OpenAPI emitter interprets this by creating a union with BarNotFoundError and telling the clients that they can expect either Bar or BarNotFoundError. This does not tell the emitter to propagate the error to the parent or the operation. Now we could just add a @throws to this and expect the emitters to understand that when @throws is added, they need to propagate the error to the parent (until they find @handle), but I think that's a bit unintuitive and if we're already adding a decorator, we might as well add which errors we want to propagate.

[steverice]

This is meant to be explained in the section within "Alternatives Considered". Can you take a look at that and say if you think it's an adequate explanation?

[swatkatz]

A followup discussion from the earlier comment:

What does the following mean?

@error
model BarNotFoundError {
     code: int32;
     message: string;
}

# Alt 1:
model Foo {
     @throws(BarNotFoundError)
     @handles(BarNotFoundError)
     bar: Bar
}

Alt 2:
model Foo {
     @throws(BarNotFoundError)
     bar: Bar | BarNotFoundError
}

Conceptually, Alt 1 means that BarNotFoundError is thrown and handled at bar. So, it would be a no-op for most protocol emitters (like openapi and graphql), but it would mean something for autogenerated code emitters like

def bar_resolver(foo: Foo, ...):
     try:
          code_to_fetch_bar_from_foo(foo) # written by developers
     except BarNotFoundError:
          log.error(f"bar not found for foo {foo})

Alt 2: IMHO this could be an error i.e. you can't both want to send the BarNotFoundError to the clients and throw it which would imply that it is either handled by or unioned at a parent.
Or at least it seems very redundant to do so.

[steverice]

Alt 1 is discussed in @throws + @handles decorator.

In this case, the error is still considered to be throwable by the model property.

So I do not think this is a no-op for most emitters; an OpenAPI operation containing Foo would report BarNotFoundError as a possible response if nothing else @handles it.

Conceptually, Alt 1 means that BarNotFoundError is thrown and handled at bar

To put a fine point on it, it does not mean that. It means that bar will handle BarNotFoundErrors that occur within Bar, but may also raise its own BarNotFoundErrors. So while technically the error is handled and is thrown at bar, the end result is that bar can throw that error.

It means:

def bar_resolver(foo: Foo, ...):
     some_code_that_can_raise_BarNotFoundError()
     try:
          code_to_fetch_bar_from_foo(foo) # written by developers
     except BarNotFoundError:
          log.error(f"bar not found for foo {foo})
     some_code_that_can_raise_BarNotFoundError()

Regarding Alt 2, this is valid. If this is the way you want your API to behave, I see no reason to stop you.
In GraphQL, this could mean that the response for the field is BarNotFoundError but that a BarNotFoundError will also appear in the errors array.

[swatkatz]

I think you have decided that throws has a higher priority than handles and I'm saying the opposite.

In general I think it is more intuitive that handles has a higher priority, because in example of GraphQL and BarNotFoundError, if there is already a union for BarNotFoundError, then how would we stop execution at that point and make BarNotFoundError appear in the errors array? We have either fulfilled the BarNotFoundError resolver or have generated a GraphQLError, but not both, which is how the GraphQL engine operates.

Anyway, that's just my opinion that I think causes confusion and dilutes the meaning of suppress or handles. Also, note that if these decorators do get accepted, they will need to be understood by a large swath of folks and need to be intuitive without folks having to read all the fine print.

[swatkatz]

Sorry to re-open the discussion about propagate, but maybe if @throws is present, then by default the property is made non-nullable, stopping at the parent which is nullable. If we want to explicitly indicate that the field is nullable, then either it shouldn't have throw or have asData or have handle to say that the property handles the error by being nullable.

The reason is I think about throws as the error has been thrown to be propagated, and it is up to the emitter to decide what to do with it.

[steverice]

Yes, I think I agree. Let me revise this.

[steverice]

Okay, on further reflection, I think I still have this right.
It comes down to the "default" way of expressing errors in a given protocol.

In an HTTP/REST API, I think we can agree that the "default" way of expressing errors is with an HTTP status code that indicates the error.

In Thrift, the "default" way is with exceptions and the throws specification.

In GraphQL, the "default" way is to add an entry to the errors array. While this is true whether the field is nullable or not, since the default for fields is nullable, I would conclude that the "default" error behavior is to make the field null and add to the errors array.

IMO, the design should then be that the use of @throws implies use of the "default" error expression for any given protocol. i.e. in HTTP, it sets a different status code on the operation. In Thrift, it is added to the throws definition of the RPC. In GraphQL, it indicates that the field is nullable.

Each of these also suggests a default "location" where the error is to be expressed. In HTTP, it's the HTTP response. In Thrift, it's the function. In GraphQL, it is at the field that throws the error. The main difference between GraphQL and these other protocols is that the error stops at its source, while in the others it "bubbles up" to the top level.

In order to customize that "location", we have the @handles decorator. In HTTP it means "don't bubble up to the HTTP response". In Thrift it means "don't bubble up to the function". In GraphQL, it actually doesn't mean anything in the default error scenario, because the error doesn't go anywhere beyond the field that throws it.

This is where the implication of @throws and @handles should end, because they are protocol-agnostic. @throws says this error must be communicated through the default error-communication mechanism, without saying anything about what that is. @handles says the error should be suppressed, without saying anything about how it should be suppressed.

So I think we then turn to protocol-specific decorators in order to customize the error-communication mechanism. In HTTP we might have some @asHeader decorator that places the error in an X-Errors response header. In Thrift we might have some @asUnion decorator that creates a union response type with the error instead of using throws. In GraphQL, we might have an @asData (or @asUnion) decorator that creates a union response type with the error instead of adding it to the errors array. Similarly, we have a @propagate decorator that causes the error to "bubble up" past the immediate field, which remember is a deviation from the "default" behavior.

---

Circling all the way back,

I think about throws as the error has been thrown to be propagated

I don't think it means that. As described above, I think it simply means "this error should be communicated". Yes, in many protocols that communication behavior is propagation (HTTP, Thrift, Python, etc). But in others (GraphQL, Golang, etc) it isn't.

It is entirely protocol-specific what the error communication mechanism is. And therefore, it is through protocol-specific decorators that the error communication mechanism should be specified / customized.

[steverice]

I should say I'm open to any change in verbiage that more precisely implies "communicate this error" instead of "propagate this error". e.g. perhaps @raises or @hasErrors instead of @throws, and/or @suppresses instead of @handles.

[swatkatz]

In that case, I think there are few reasons why the reader walks away of thinking @throws and @handles. One is the reliance of explaining these in terms of programming languages where errors are propagated (even in Go, they are part of the return type and get sent to the caller) and another is as you suggest the naming itself. I think it would benefit the proposal to reconsider the language when explaining the proposal and re-writing it in terms of @hasError is communicating that an error can happen and the location where it can happen and @handles is communicating where an error will be handled in some way and the location where it will be handled.

Each protocol will treat @handles the way it wants. E.g. the operation in OpenAPI will not have an error union if there is handles, but the same may not be true for GraphQL.

In addition, sections like:

Operation errors + @throws decorator and similar for handles muddy the waters as they treat operations special and make the reader think that the error is somehow reaching or propagating to that operation and are HTTP/OpenAPI emitter centric.

I think it would benefit the proposal from just talking about the decorators and what they mean and then showing examples of how each emitter can handle them differently.

[swatkatz]

It is important to note here that we don't want to model these errors as unions because we're not wanting to change the shape of TSP based on this metadata that can be handled by the emitters differently.

[steverice]

Additional feedback from @swatkatz:

• steer readers away from the idea that @throws implies a method of communicating errors (i.e. propagation) and reframe it around stating that "an error happens here"
• @handles is probably okay
• @throws is confusing because it implies error handling behavior
• nuance around @throws + @handles with the same error — @throws always wins? do emitters need to know about this (yes, code emitters do)?
• code examples are putting too much emphasis on implied propagation
• operation return types with errors is OpenAPI-specific, not TypeSpec-generic
• return types are a valid alternative (they are not in conflict with @throws) but they mean something different