microsoft / typespec

https://typespec.io/
MIT License
4.56k stars 223 forks source link

Global defaultResponse/error #3082

Open happenslol opened 8 months ago

happenslol commented 8 months ago

Clear and concise description of the problem

It would be great if it was possible to globally define an Error type that all operations (or maybe just all operations within a namespace) use as a default response for OpenAPI.

This would enable defining something like a ServerError type that acts as a fallback, instead of having to use Thing | NotFoundResponse | ServerError for every operation.

I'm not sure what this would look like code-wise, but I can try to come up with some imaginary code examples if you'd like to explain the feature request further.

Checklist

timotheeguerin commented 8 months ago

One of the issues with having configurable default behavior in typespec is when using op is, model is, ..., etc. that doesn't cary over the context of where things were defined(due to decorators running bottom up). We already see this issue when providing common @route on a namespace or interface and then trying to op is a containing operation then the information is lost.

For example

@defaulError(SomeLibServerError)
namespace SomeLib;

op read(): string; // When looking at this operation we could see this being `string | SomeLibServerError`

// in your service
namespace MyService;

op read is SomeLib.read; // now this is not what I was looking for as it is just `string`

The pattern we have been prefering is defining template operation

op MyServiceOp<T>(): T | SomeLibServerError;

op read is MyServiceOp<string>;
happenslol commented 8 months ago

Ah, I understand. That's unfortunate, and seems a bit unintuitive. Since this is already causing problems in other spaces, is it something that you're planning on changing in the long run?

timotheeguerin commented 8 months ago

We unfortunately don't have any plan to change that for now, as much as this case could benefit from having this, not all decorators would want to be carried over(so would want some sort of modifier here as well. In the @route case it could even depend on how you are using it if you want it to carry over or not). On top of that I find it can also lead to some more confusing understanding of where are things coming from.

There is some more or less hacky ways we can achieve the behavior you are looking for with the current system, it could also just not be something we recommend using in a library(which would mean we ignore the issue). But I do feel like we might want more feedback of this being a wanted/needed feature before considering it.

bterlson commented 8 months ago

Just chiming in to offer that you can also use a templated union, ala:

alias WithStandardErrors<T> = T | StandardErrorResponse;

op read(): WithStandardErrors<string>;

It is still verbose compared to a global setting, but it doesn't require an op template which can give some additional flexibility.

allenjzhang commented 4 months ago

Thanks for filing the issue. Considering the tradeoff, the proposed solution sounds like an adequate solution.

happenslol commented 4 months ago

I'm currently using Result<...> for every operation response. It's unfortunate that I have to do it for absolutely every operation, which feels like boilerplate that could be avoided, but at least it's a working solution.