smartcar / python-sdk

Smartcar Python SDK
MIT License
44 stars 13 forks source link

BUG: retry_after missing in Rate Limit SmartcarExceptions that occur on individual endpoints when making a batch call #135

Closed Silidrone closed 3 months ago

Silidrone commented 4 months ago

I am using smartcar==6.10.1 python package.

When calling the batch function passing a list of endpoints, if a rate limit smartcar.exceptions.SmartcarException occurs, the exception is always missing the retry_after key, which is supposed to be taken from the Retry-After header.

Below is one example where I made a request which gets rate limited and returns a Retry-After header, but the exception does not have the retry_after key. The request id is: 58ae060d-0a03-40a4-a628-3b699b479cd7 I figured out why the problem is happenning, I am happy to open a PR for the fix if I am on the right track here:

When the batch function is called, each passed endpoint's response is checked whether it has a HTTP status different than 200. Of course the endpoint with the request-id given above has a status of 429 (rate limit), and the batch function invokes the exception factory, constructs the exception and assigns it to be raised when that specific attribute is later called on the batch result object. The problem is in the batch function (which is found in vehicle.py:400), when it invokes exception_factory, it passes the headers of the batch request itself, not the headers of the individual endpoint that failed. I can actually see in code that here (vehicle.py:457): sc_exception = sce.exception_factory(code, headers, body) headers are missing the Retry-After header, but res_dict.get("headers") actually is {'Retry-After': 80} res_dict is the individual response for each of the invoked endpoint within the batch, not the batch's response (which is always 200) So I tested when I change vehicle:457 from: sc_exception = sce.exception_factory(code, headers, body) to: sc_exception = sce.exception_factory(code, {**headers, "Retry-After": res_dict.get("headers").get("Retry-After")}, body) My smartcar.exceptions.SmartcarException object has a present retry_after attribute.

Silidrone commented 3 months ago

Fixed in #136