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

Client methods should be able to return custom errors #1995

Open
heaths opened this issue Jan 15, 2025 · 2 comments
Open

Client methods should be able to return custom errors #1995

heaths opened this issue Jan 15, 2025 · 2 comments
Labels
Azure.Core The azure_core crate Client This issue points to a problem in the data-plane of the library. unbranded Issues concerning client generation that are not required for Azure services.

Comments

@heaths
Copy link
Member

heaths commented Jan 15, 2025

Currently, our Rust guidelines dictate returning an azure_core::Result<T> from our client methods, which is re-exported from typespec which aliases this as std::result::Result<T, typespec::error::Error> where typespec::error::Error is modeled after our REST API guidelines.

For TypeSpec-emitted clients that do no return an Azure error, we should support returning custom errors even if basic i.e., just an HTTP status code.

One idea is that we do this unconditionally if emitting a client in an "unbranded" mode.

Another idea would be to just emit code that returns a std::result::Result<T, E> where E is the modeled error type IIF E isn't modeled after an Azure error response. Note that typespec::error::Error does not mention "Azure" - it's a generic definition of Azure error specs.

@heaths heaths added Azure.Core The azure_core crate Client This issue points to a problem in the data-plane of the library. unbranded Issues concerning client generation that are not required for Azure services. labels Jan 15, 2025
@github-project-automation github-project-automation bot moved this to Untriaged in Azure SDK Rust Jan 15, 2025
@heaths
Copy link
Member Author

heaths commented Jan 15, 2025

Once we start buffering the entire response in the pipeline we're probably going to need to radically rethink what we return from the pipeline i.e., that it can't just be a typespec::Result<T> but the typespec::http::Response<T> so that the upstream client - whether for Azure services or not - can handle the error. I believe this is what some of the other languages are doing: helpers for Azure services to parse and "throw" errors, but keeping their lower-level pipelines simple.

/cc @JeffreyRichter @RickWinter

@heaths
Copy link
Member Author

heaths commented Jan 16, 2025

I was thinking about this more and talking with @lmazuel and I think returning a typespec::error::Error is actually fine IIF we hae a way to supply a list of success and error codes (any code not in the success list should probably be an error since it's not spec'd) OR - and I'm leaning toward this if not too bad on perf - a handler function that emitters and SDK devs can supply that categorize responses based primarily (perhaps only) on HTTP status codes. We can provide a default for Azure clients when emitting code.

This is basically what .NET is doing with their DPG methods - or at least was when I was on that team. The code generator folded and emitter delegates to categories whether a 200 or 202 or whatever was expected, and everything else was an error. And for Azure services, this is probably best - and makes the case for a single handler method - because the REST API Guidelines state,

⚠️YOU SHOULD NOT document specific error status codes in your OpenAPI/Swagger spec unless the "default" response cannot properly describe the specific error response (e.g. body schema is different).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core The azure_core crate Client This issue points to a problem in the data-plane of the library. unbranded Issues concerning client generation that are not required for Azure services.
Projects
Status: Untriaged
Development

No branches or pull requests

1 participant