search

dlt-hub / dlt

data load tool (dlt) is an open source Python library that makes data loading easy 🛠️
https://dlthub.com/docs
Apache License 2.0
2.73k stars 181 forks source link

show full response when printing failed api calls #1605

Open shuaahvee opened 4 months ago

shuaahvee commented 4 months ago

Feature description

If an API call fails, the cli calls the requests library's raise_for_status method which prints the request's status code and reason. But not all APIs use the response's reason attribute. It would be helpful if we could instead either configure what response attributes get printed or at least print response.text. E.g. I was debugging a klaviyo connector and got the usual response message when my api call failed. The cause was that I hadn't configured the request header correctly but the only way I figured that out was by editing the raise_for_status method's code to look like this: if 400 <= self.status_code < 500: http_error_msg = ( f"{self.status_code} Client Error: {reason} for url: {self.url}. Full response: {self.text}" )

Are you a dlt user?

Yes, I'm already a dlt user.

Use case

debugging failed klaviyo api calls

Proposed solution

override the raise_for_status method? or maybe add a config that lets you specify which response attributes should be printed on failure

Related issues

No response

shuaahvee commented 4 months ago

Worth pointing out that there are potential security concerns with printing a full response on error. However it looks like certain methods already do that...

burnash commented 4 months ago

Hey @shuaahvee thanks for suggestion. It's very important for us to let users know why the API call failed. Are you using rest_api source or one of our Requests wrappers?

shuaahvee commented 4 months ago

rest_api_source!

willi-mueller commented 2 months ago

Hey @shuaahvee , thank you for your feature request! We have implemented it here: https://github.com/dlt-hub/dlt/pull/1895

I'd be happy to hear your feedback!

burnash commented 2 weeks ago

@shuaahvee thanks again for reporting this issue. And also for noticing that leaks full response in some places – we're going to address that in a separate PR.

@willi-mueller @shuaahvee I propose a different solution than logging: https://github.com/dlt-hub/dlt/pull/1895#issuecomment-2467658933 Could you take a look and share if it could be useful for your case?