Skip to content

Add exceptions for invalid trace callback - #717

Merged
karpetrosyan merged 6 commits into
encode:masterfrom
karpetrosyan:exception-for-trace-function
Jun 12, 2023
Merged

Add exceptions for invalid trace callback#717
karpetrosyan merged 6 commits into
encode:masterfrom
karpetrosyan:exception-for-trace-function

Conversation

@karpetrosyan

Copy link
Copy Markdown
Contributor

Examples

importhttpcoreasyncdeflog(a, b):
print(a, b)
httpcore.request("GET", "https://google.com", extensions={"trace": log})

Old output

RuntimeWarning: coroutine 'log' was never awaited
self.trace_extension(prefix_and_name, info)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

New output

TypeError: If you are using a synchronous interface, the callback of the `trace` extension should be a normal function instead of an asynchronous function.
importtriodeflog(a, b):
print(a,b)
asyncdefreq():
cl=httpcore.AsyncConnectionPool()
awaitcl.request("GET", "https://google.com", extensions={"trace": log})
trio.run(req)

Old output

TypeError: object NoneType can't be used in 'await' expression

New output

TypeError: If you're using an asynchronous interface, the callback of the `trace` extension should be an asynchronous function rather than a normal function.

@karpetrosyan

Copy link
Copy Markdown
ContributorAuthor

When migrating from synchronous to asynchronous interface, it's common to forget to change the synchronous trace callback to an asynchronous function, so let's add a user-friendly behavior for those cases.

@karpetrosyan
karpetrosyan requested a review from a teamJune 10, 2023 08:18
Comment threadhttpcore/_trace.py Outdated

@lovelydinosaurlovelydinosaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

@karpetrosyan
karpetrosyan merged commit 7548dd5 into encode:masterJun 12, 2023
@karpetrosyankarpetrosyan mentioned this pull request Jul 3, 2023
1 task
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karpetrosyan@lovelydinosaur@zanieb