[corehttp] HttpResponseError lacks kwargs signature model compared with same class of azure-core

[msyyc]

HttpResponse lacks kwargs signature model compared with same class of azure-core

• corehttp:
  

• azure-core:
  

[github-actions]

@kashifkhan @xiangyan99

[github-actions]

Thank you for your feedback. Tagging and routing to the team member best able to assist.

[msyyc]

Not sure whether it is by design, but I think model signature is important. For example, when Typespec defines customized error like:

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

(typepsec link)
The generated SDK will try to deserialize the response body with PetStoreError when error occurs then set it in model signature

            map_error(status_code=response.status_code, response=response, error_map=error_map)
            error = _failsafe_deserialize(_models.PetStoreError, response.json())
            raise HttpResponseError(response=response, model=error)

(generated SDK link)

so that SDK users could access the user-defined error model easily like:

try:
   client.op(...)
except HttpResponseError as err:
    print(err.model.code)
    print(err.model.message)

However, if corehttp doesn't have model signature for HttpResponseError, there is no other way to set the deserialized model for SDK users.

[pvaneck]

Hey @msyyc, sorry for the delay in getting to this. I don't recall the specific reasoning for why model was removed from HttpResponseError in corehttp. Potentially, it was just to keep things slim initially. However, we can definitely look into adding it if it would provide some use. Some questions I have are:

1. What would the model be typed as? Any? In azure-core, we have msrest.serialization.Model listed as the type, however that's certainly not applicable here.
2. Is reading the model from the HttpResponseError commonly done by users? Seems the end user would need to know the model and model fields ahead of time, as I don't think Intellisense would help with that since the typing is likely arbitrary.

[msyyc]

Hey @msyyc, sorry for the delay in getting to this. I don't recall the specific reasoning for why model was removed from HttpResponseError in corehttp. Potentially, it was just to keep things slim initially. However, we can definitely look into adding it if it would provide some use. Some questions I have are:

1. What would the model be typed as? Any? In azure-core, we have msrest.serialization.Model listed as the type, however that's certainly not applicable here.
2. Is reading the model from the HttpResponseError commonly done by users? Seems the end user would need to know the model and model fields ahead of time, as I don't think Intellisense would help with that since the typing is likely arbitrary.

1. I think it should be MutableMapping[str, typing.Any] which is parent class of Model in _model_base.py: https://github.com/Azure/autorest.python/blob/9519669e60300c06a8052db1b90e3a4ba1c6cdd0/packages/typespec-python/test/azure/generated/routes/routes/_model_base.py#L540
2. Without the model, users have to access specific attribute like response_error.response.json()["xxx"]. Compared with response_error.xxx, it is a little not convenient. As for IntelliSense, both ways can't provide IntelliSense. Above all, model could provide better usage experience I think. @iscai-msft / @tadelesh for awareness.

[tadelesh]

one real usage for unbranded sample code: https://github.com/allenjzhang/typespec-e2e-demo/blob/a9759159241d876891e31c20bf24009160121965/todoApp/clients/python/test/test_todo_app.py#L62

[pvaneck]

Created a PR: #38956, but still a bit unsure of the typing for model. Typing it as a Mapping/MutableMapping would cause type-checkers/linters to complain if a user tries to access info using attribute access (i.e. err.model.some_attribute instead of err.model["some_attribute"]). Opting to keep it as Any for now unless we can find a better option.