Support status code range for errors

[lmazuel]

Example for TypeSpec like:

@jsonSchema
@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;
}

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

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

model Standard5XXResponse extends ApiError {
  @minValue(500)
  @maxValue(599)
  @statusCode
  statusCode: int32;
}

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

We should generate the code:

        if response.status_code not in [200]:
            map_error(status_code=response.status_code, response=response, error_map=error_map)
            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)
            elif 500 <= response.status_code <= 599:
                error = self._deserialize.failsafe_deserialize(_models.Standard5XXResponse, pipeline_response)
            raise HttpResponseError(response=response, model=error)

It should be for both unbranded and Azure, as a fundational feature of our codegen

[lmazuel]

See microsoft/typespec#5203 (comment) for additional details

[msyyc]

Since python SDK has error_map predefined, we could add all the status code there. For example:

for status_code in range(400, 499):
   error_map.update[status_code] = cast(
        Type[HttpResponseError],
        lambda response: HttpResponseError(
            response=response, model=_deserialize(_models.Standard4XXResponse, response.json())
        ),
    ),

With this way, we don't need to change error handling logic.

nit: DPG model serialize doesn't have failsafe_deserialize for now.

[tadelesh]

Since python SDK has error_map predefined, we could add all the status code there. For example:

for status_code in range(400, 499):
error_map.update[status_code] = cast(
Type[HttpResponseError],
lambda response: HttpResponseError(
response=response, model=_deserialize(_models.Standard4XXResponse, response.json())
),
),
With this way, we don't need to change error handling logic.

nit: DPG model serialize doesn't have failsafe_deserialize for now.

it is the easiest way, but the code is so duplicate. maybe we could vendor a map_error in package to support ranged status code?

[lmazuel]

I don't understand why we're touching error_map? Error_map is to pick the exception, it's not to change the model that we deserialize. It also would overwrite what would have been passed by the caller, which we shouldn't do. I think the error_map solution is fixing it the wrong way.

[msyyc]

Error_map is to pick the exception, it's not to change the model that we deserialize.
-> If we look into the solution, we can see both way actually have same behavior when error occurs.

It also would overwrite what would have been passed by the caller, which we shouldn't do.
-> error_map is predefined and callers can still overwrite it in https://github.com/msyyc/typespec-todo/blob/ac50ff69ebdbcce3aa2a4b3fd3edeffe028c5347/output-python/todo/aio/operations/_operations.py#L254

Anyway, I think both solutions result in same behavior and @tadelesh could pick up one.

[msyyc]

Since python SDK has error_map predefined, we could add all the status code there. For example:
for status_code in range(400, 499):
error_map.update[status_code] = cast(
Type[HttpResponseError],
lambda response: HttpResponseError(
response=response, model=_deserialize(_models.Standard4XXResponse, response.json())
),
),
With this way, we don't need to change error handling logic.
nit: DPG model serialize doesn't have failsafe_deserialize for now.

it is the easiest way, but the code is so duplicate. maybe we could vendor a map_error in package to support ranged status code?

We could also create a helper function in _vendor.py to make it more clean. For example:

# operations.py
...
error_map.udpate(get_error_map({(401, 499): _models.Standard4XXResponse}, (501,599): _models.Strandard5XXResponse))

# _vendor.py
error_map = {}
def get_error_map(error_info: Dict[Tuple[int,int], Any])
    for status_code_range, model_type in error_info:
        for status_code in range(status_code_range[0], status_code_range[1]):
               error_map[status_code] = cast(
                  Type[HttpResponseError],
                  lambda response: HttpResponseError(
                      response=response, model=_deserialize(model_type, response.json())
                  ),
              ),

[tadelesh]

I don't understand why we're touching error_map? Error_map is to pick the exception, it's not to change the model that we deserialize. It also would overwrite what would have been passed by the caller, which we shouldn't do. I think the error_map solution is fixing it the wrong way.

in current sdk, we use map_error to deserialize the user-defined error. it is both for swagger x-ms-error-response (example) and typespec @error response (example). so, we are thinking of enhance the map_error to support ranged status code. then the main error handling logic will not be changed. user could also overwrite the error_map with ranged status code.