WebSocket Fix Proposal

[fatualux]

Description

This is a proposal to fix this issue.

FIX 1

The first step is to edit stray_cat.py:

Basically, we add a try-except block to the original code (run method) to handle the problem:

from websockets.exceptions import ConnectionClosedOK

    def run(self, user_message_json, return_message=False):
        try:
            cat_message = self.loop.run_until_complete(self.__call__(user_message_json))
            if return_message:
                # return the message for HTTP usage
                return cat_message
            else:
                # send message back to client via WS
                self.send_chat_message(cat_message)
        except Exception as e:
            log.error(e)
            traceback.print_exc()
            if return_message:
                return {"error": str(e)}
            else:
                try:
                    self.send_error(e)
                except ConnectionClosedOK as ex:
                    log.warning(ex)
                    if self.__ws:
                        del self.__ws
                        self.__ws = None

It catches the ConnectionClosedOK exception, and, it logs a warning using log.warning(ex).

Then, if the ConnectionClosedOK exception is caught, it deletes the self.__ws variable and sets it to None.

In the screencast below we can observe how the fix works against reproduce.py:

FIX 2

The first fix is used to handle this specific case: a given user closes the connection while the Cat is still providing an answer.

In case of a hypothetical different flow, we would run into the same problem again.

To avoid this, we can generalize the fix, adding this try-except block to the async method get_user_stray in connection.py:

    if stray._StrayCat__ws:
    try: 
        await stray._StrayCat__ws.close()
    except RuntimeError as ex:
        log.warning(ex)
        if stray._StrayCat__ws:
            del stray._StrayCat__ws
            stray._StrayCat__ws = None

This fix ensure that, if we get a RuntimeError from WebSocket, we get a warning, and the stray, that once again is an inconsistent state, is cleaned up (same strategy we used for the first fix).

To avoid using a private attribute, we could extract this and implement a new method in the StrayCat class:

• stray_cat.py

    async def close_connection(self):
        if self.__ws:
            try:
                await self.__ws.close()
            except RuntimeError as ex:
                log.warning(ex)
                if self.__ws:
                    del self.__ws
                    self.__ws = None

• connection.py

        if user.id in strays.keys():
            stray = strays[user.id]
            await stray.close_connection()

In the screencast below, again, we can see how the second fix works against reproduce.py:

Related to this issue

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas

I did not commented my code, I think it is not needed in this case.

As always, thanks for your work and support/review.

[fatualux]

UPDATE

As @pieroit asked, I branched off develop, and, as @sambarza suggested, I moved the exception handling into the run method.

Initially, I had added an if statement to catch ex.code == 1000 , but, during my tests, i got into other error codes too (1001 and 1005).

So I decided to generalize my try-except block.

As always, thanks for your work and your support

[pieroit]

Thanks @fatualux !!!
Welcome