Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undocumented behaviour / incorrect type hints #125

Open
mheguy opened this issue Jul 14, 2024 · 0 comments · May be fixed by #126
Open

Undocumented behaviour / incorrect type hints #125

mheguy opened this issue Jul 14, 2024 · 0 comments · May be fixed by #126

Comments

@mheguy
Copy link

mheguy commented Jul 14, 2024

There is no documentation indicating that ngrok.forward can return an _asyncio.Task.

Running this code:

import asyncio
import ngrok
NGROK_TOKEN = ""

def sync():
    listener = ngrok.forward(80, authtoken=NGROK_TOKEN)
    print("sync")
    print(type(listener))

async def async_():
    listener = ngrok.forward(80, authtoken=NGROK_TOKEN)
    print("async")
    print(type(listener))

    print("async calling sync..")
    sync()

sync()
asyncio.run(async_())

Gives this output:

sync
<class 'builtins.Listener'>
async
<class '_asyncio.Task'>
async calling sync..
sync
<class '_asyncio.Task'>

After a bit more exploring, all methods belonging directly to the ngrok module have the same behaviour.

Either:

  • the type hints should be updated to reflect the true return types
  • explicit async methods should be implemented (ex. forward_async)

The second option is a better solution.
As my example hints at: the sync function calls forward and gets back an awaitable.
This is problematic because the sync method cannot await the listener. This means that it has to be refactored into an async function. It also means that the caller of sync either needs to be async itself, or run the function explicitly with asyncio.run() (or similar).

Edit: On reflecting on this, it might be a bug in how the code is detecting whether it should return an awaitable or not. That isn't a question I can speak to intelligently.

@mheguy mheguy linked a pull request Jul 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant