[Unbranded Client CodeGen]: Support generating error models instead of reusing the model from core library

[lirenhe]

Clear and concise description of the problem

For unbranded client generation, today, we still use the models from core library when we see the following code which is not correct.
In the unbranded scenario, we should generate models for each Error decorated by @error .

@error
model ApiError {
  /** A machine readable error code */
  code: string;

  /** A human readable message */
  // https://github.com/microsoft/OpenAPI/blob/main/extensions/x-ms-primary-error-message.md
  @OpenAPI.extension("x-ms-primary-error-message", true)
  message: string;
}

list of item for each language

Preview Give feedback

1. Unbranded CodeGen should generate error models instead of reusing the model from core library Azure/autorest.python#2937
   +0
   
2. Unbranded CodeGen should generate error models instead of reusing the model from core library Azure/autorest.java#2991
   DPG +1
   
3. Unbranded CodeGen should generate error models instead of reusing the model from core library Azure/autorest.csharp#5183
   v3 +1
   

   
4. Unbranded CodeGen should generate error models instead of reusing the model from core library Azure/autorest.typescript#2931
   1_0_E2E HRLC P1 non-azure +4
   

Checklist

• Follow our Code of Conduct
• Read the docs.
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

[MaryGao]

@lirenhe @lmazuel I'd like to clarify the requirement here, when we talk about supporting error model generation, do we talk about the following 1 or both?

• first we need to generate these customized error models and expose to end users
• second our client should serialize to that error model if the error code is matching

[lirenhe]

@lirenhe @lmazuel I'd like to clarify the requirement here, when we talk about supporting error model generation, do we talk about the following 1 or both?

• first we need to generate these customized error models and expose to end users
• second our client should serlize to that error model if the error code is matching

It should be both, but currently, we don't generate error models so I am not sure the status of (2).
For those issues created for each emitter, I already added the verification of the range of the status code.

[MyNameIsNeXTSTEP]

@lirenhe Hi!
Can you please provide some information about the "verification of the range of the status code" and how to use it ?

Another question about generating erros models, and consider I'm new to this spec:
Could somehow an error model for the end users be described in the .tsp spec as a normal model, where after that the actual decaring of @error model would extend it or reuse as outer model ?

[lmazuel]

Spent some time today to confirm with @bterlson and @johanste

Today we consider errors based on a constant, but if intervals are applied to status code, we should be able to use them. For instance:

model Standard4XXResponse extends ApiError {
  @minValue(400)
  @maxValue(499)
  @statusCode
  statusCode: int32;
}

Means clearly "if in that range, use this model".

I create a Python issue with the Python design, linking it here if that helps:
Azure/autorest.python#2940

This may requires some TCGC help, TBD with @tadelesh

We should support only intervals through minValue/maxValue and constant, nothing else

[lmazuel]

Continued the talk, and ended up with this:

P0: "do no harm" behavior.

A client language can decide to ignore custom error type if it's not idiomatic to the language, and if their core library doesn't expose custom error deserialization to customers through exception.
In that case, the models must not be generated, as they are not used by anything.
For E2E. it's ok if we stop here for now.

P1: Support for custom error model

A client language can decide to parse an error received from the service according an error model defined in the TypeSpec. The right model can be discriminated the following way:

• It uses a constant, like @statusCode statusCode: 400;
• It uses an interval, like @statusCode @minValue(400) @maxValue(499) statusCode int32;
• A model is tagged @error, but no status code constraints are provided. In this case, it counts as a default. Note that errors code are always between 400 and 599.

Example of TypeSpec with those scenarios:

@error
model ApiError {
  /** A machine readable error code */
  code: string;

  /** A human readable message */
  message: string;
}

model Standard400Response extends ApiError {
  @statusCode
  statusCode: 400;
}

model Standard4XXResponse extends ApiError {
  @minValue(401)
  @maxValue(499)
  @statusCode
  statusCode: int32;
}

alias WithStandardErrors<T> = T | Standard400Response | Standard4XXResponse  | ApiError

Translate as:

• If error code is 400, deserialize using Standard400Response
• If error code is between 401 and 499, deserialize using Standard4XXResponse
• For everything else, use ApiError (in practice, 500 to 599 given the previous two constraints)

This would translate to the following Python code (to illustrate):

        if response.status_code not in [200]:
            error = None
            if response.status_code == 400:
                error = self._deserialize.failsafe_deserialize(_models.Standard400Response, pipeline_response)
            elif 401 <= response.status_code <= 499:
                error = self._deserialize.failsafe_deserialize(_models.Standard4XXResponse, pipeline_response)
            else:
                error = self._deserialize.failsafe_deserialize(_models.ApiError, pipeline_response)
            raise HttpResponseError(response=response, model=error)

Note:

• Union are not ordered in TypeSpec, so we need to pass through the union to check if there is a default error model, to be sure it's used last
• Garbage in garbage out. It's not the responsibility of client emitter to check if the constant/range described cover all errors, or not overlap
• Deserializing error model must be a failsafe operation. If a deserialization error occurs, the original exception must be raised without the error model. There are many situations where the received payload may be incorrect (misconfigured proxy, HTML nginx error page, etc.) and we shall NOT substitute a server error with a deserialization error

[weidongxu-microsoft]

Agree to above.

There are a few cases in typespec-todo

@get op get(@path id: TodoItem.id): TodoItem | NotFoundResponse;

https://github.com/bterlson/typespec-todo/blob/9ef84051c675b913f0300bc66bbc876c9150580b/main.tsp#L254

It produces

• 200 with response body TodoItem as expected
• 404 without body as expected
• all other status code as error but without model

---

  @delete op delete(
    @path id: TodoItem.id,
  ): WithStandardErrors<NoContentResponse | NotFoundResponse>;

alias WithStandardErrors<T> = T | Standard4XXResponse | Standard5XXResponse;

https://github.com/bterlson/typespec-todo/blob/9ef84051c675b913f0300bc66bbc876c9150580b/main.tsp#L260-L262

It produces

• 204 without body as expected
• 404 without body as expected
• 4xx (but exclude 404) as error with model Standard4XXResponse
• 5xx as error with model Standard5XXResponse
• all other status code (include not specified 2xx and 3xx) as error but without model

---

There is also a nuance about Standard4XXResponse

model Standard4XXResponse extends ApiError {
  @minValue(400)
  @maxValue(499)
  @statusCode
  statusCode: int32;
}

On this tsp, there seems no need to have another Standard4XXResponse as model in generated code (it only adds the status code range to ApiError). But if we slight tweak it by adding a property, it is now needed.

An alias may be more appropriate, if we don't need the Standard4XXResponse model.

alias Standard4XXResponse = ApiError & {
  @minValue(400)
  @maxValue(499)
  @statusCode
  statusCode: int32;
};