Skip to content

AbstractSyncWebSocketClient.send return True even if msg not received #510

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

Open
obo-cell opened this issue Mar 17, 2025 · 1 comment
Open

Comments

@obo-cell
Copy link

Proposed changes

I would like to suggest a change to implementation of AbstractSyncWebSocketClient.send method. It should return True if & only when the send() is succeeded. Why True is returned in case when websockets.exceptions.ConnectionClosedOK or websockets.exceptions.ConnectionClosed?

Maybe, I dont understand the sense of bool value returned by send, but for me if user (deepgram) didn't get any message I expect to get False

Context

File: deepgram/clients/common/v1/abstract_sync_websocket.py
Class: AbstractSyncWebSocketClient
Method: send
Description:

    def send(self, data: Union[str, bytes]) -> bool:
        """
        Sends data over the WebSocket connection.
        """

Possible Implementation

                except websockets.exceptions.ConnectionClosedOK as e:
                    self._logger.notice(f"send() exiting gracefully: {e.code}")
                    self._logger.debug("AbstractSyncWebSocketClient.send LEAVE")
                    if self._config.options.get("termination_exception_send") is True:
                        raise
                    return False
                except websockets.exceptions.ConnectionClosed as e:
                    if e.code in [1000, 1001]:
                        self._logger.notice(f"send({e.code}) exiting gracefully")
                        self._logger.debug("AbstractSyncWebSocketClient.send LEAVE")
                        if (
                            self._config.options.get("termination_exception_send")
                            == "true"
                        ):
                            raise
                        return False

Other information

@obo-cell
Copy link
Author

Does my suggestion make sense?

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

No branches or pull requests

1 participant