deepgram / deepgram-python-sdk

Official Python SDK for Deepgram's automated speech recognition APIs.
https://developers.deepgram.com
MIT License
239 stars 63 forks source link

Socket exceptions are not caught in async live client #307

Closed leddy231 closed 8 months ago

leddy231 commented 9 months ago

What is the current behavior?

In the AsyncLiveClient there is an exception handler that tries to catch exceptions coming from the socket, and to send those to any registered error handlers.

https://github.com/deepgram/deepgram-python-sdk/blob/61feac2c6a4604f2b758e40562f8fe1552bb6f81/deepgram/clients/live/v1/async_client.py#L213-L241

However, this is inside the try-except block inside the async for loop at the top

https://github.com/deepgram/deepgram-python-sdk/blob/61feac2c6a4604f2b758e40562f8fe1552bb6f81/deepgram/clients/live/v1/async_client.py#L119

The socket exceptions are raised from the top of the loop, so they are outside this try-except block. The error handlers therefore never get called

Steps to reproduce

Hard to make a small example since you need a error from deepgrams side

Expected behavior

I would expect the exceptions to be caught and sent to the defined error handler

Please tell us about your environment

linux python 3.10.4 deepgram sdk 3.1.4

Other information

It can be hard to spot the logic in the code, but the issue is equivalent to this small example

async def asyncGenerator():
    for i in range(3):
        yield i
    raise Exception("Error")

async def main():
    async for i in asyncGenerator():
        try:
            print(i)
        except Exception as e:
            print(f"Caught exception: {e}")
            break

async def working_main():
    try:
        async for i in asyncGenerator():
            print(i)
    except Exception as e:
        print(f"Caught exception: {e}")

if __name__ == "__main__":
    import asyncio

    asyncio.run(main())

main() has the try-except inside the for loop, which does not work. working_main has it outside, which works

davidvonthenen commented 9 months ago

Yup, someone just caught this in the LiveClient in that case it exposed it's self in the form a deadlock. Will need to do something similar for Async. https://github.com/deepgram/deepgram-python-sdk/pull/305

Thanks for reporting this. Currently working on Async to address these issues https://github.com/deepgram/deepgram-python-sdk/issues/272 and https://github.com/deepgram/deepgram-python-sdk/issues/301 so we should have something shortly.

davidvonthenen commented 9 months ago

This PR should address this issue: https://github.com/deepgram/deepgram-python-sdk/pull/309

davidvonthenen commented 8 months ago

PR Merged. Closing.