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

Passing method of non-WebSocketApp object as callback does not receive the WebSocketApp object as an argument #499

Closed
rmah opened this issue Nov 2, 2018 · 9 comments

Comments

@rmah
Copy link

rmah commented Nov 2, 2018

Previous versions of websocket-client allowed passing methods of non-WebSocketApp objects as callbacks and have them receive the WebSocketApp object as the first argument. This was very convenient.

Recent versions (0.53, 0.54) introduced a change to _callback in _app.py that checks if the callback is a method. And if so, not pass the WebSocketApp object to the callback. This is fine if the callback method is bound to a WebSocketApp subclass, but very annoying if not.

Passing bound methods of other classes as callbacks is a very common pattern and it would be great if websocket-client could re-enable the original behavior for such cases.

A fix would be very simple, just one line of code:

    def _callback(self, callback, *args):
        if callback:
            try:
                if inspect.ismethod(callback) and isinstance(callback.__self__, type(self)):
                    callback(*args)
                else:
                    callback(self, *args)

This change would allow 0.53/0.54 behavior for methods of WebSocketApp subclasses and 0.52 behavior for methods of objects that are not WebSocketApp subclasses.

As an aside, the older behavior seems more common in python modules with callbacks. I can understand the desire for the 0.53 change, but I think the more typical pattern is to have the subclass methods get called directly (i.e., have on_open, on_message, etc. methods in WebSocketApp that don't do anything and are overridden in subclasses) if no callback attribute is specified. But that is, of course, up to the author.

Thanks

@icehongssii
Copy link

@liris or indeed, Could you modify the example? it made me confused.

@rmah
Copy link
Author

rmah commented Nov 21, 2018

An example of what we are talking about is when someone does something like this:

from websocket import WebSocketApp

class MyApp (object):
    def ws_open(self, ws):
        print("Connected")
        ws.send("Hello world")

    def ws_message(self, ws, msg):
        print("Got message: %s" % msg)
        ws.close()

if __name__ == '__main__':
    app = MyApp()
    url = 'wss://echo.websocket.org'
    ws = WebSocketApp(url, on_open=app.ws_open, on_message=app.ws_message )
    ws.run_forever()

Note that MyApp is not a subclass of WebSocketApp. This pattern is common and allows for greater flexibility in structuring applications.

In my applications, we have a multiple handlers for third party services, which may or may not have websocket support. Thus servivce classes should not subclass from WebSocketApp. Instead, specific protocol handlers get created (websocket among them) as necessary for each service. Each protocol handler initiates callbacks to methods in the service handler object which normalizes messages for use by the service object.

The lack of the websocket object being passed to event handlers when they are methods can, obviously, be worked around. However, it's less than optimal to put "if websocket.version > xyz: do_this() else: do_that()" to work around the changes introduced in how websocket-client executes callbacks in 0.53.

Hope this clears it up.

@pbechliv-zz
Copy link

I had the exact same problem when i tried upgrading from version 0.48 to 0.54

@icehongssii
Copy link

@pbechliv just use 0.52.0. the changes starts from 0.53.0

@jhtitor
Copy link
Contributor

jhtitor commented Dec 19, 2018

+1, this is very inconvenient, #510, #514 are the same fixes in PR form.

@akshayeshenoi
Copy link

I'm curious, why was this change introduced in the first place?

@ajgbarnes
Copy link

I got around this issue for now using a lambda function so, for example:

...
ws = WebSocketApp(url, 
   on_open=lambda ws, msg: app.ws_open(ws), 
   on_message=lambda ws, msg: app.ws_message (ws, msg),
   on_error=lambda ws, msg: app.ws_error(ws, msg),
   on_close=lambda ws: app.ws_close(ws))
...

I hope this helps someone

@yemreak
Copy link

yemreak commented Jun 19, 2020

Hello I tried to use @ajgbarnes code, but got some error so I change code like this:

ws = WebSocketApp(url, 
   on_open=lambda ws: app.ws_open(ws), # <--- This line is changed
   on_message=lambda ws, msg: app.ws_message (ws, msg),
   on_error=lambda ws, error: app.ws_error(ws, error), # Omittable (msg -> error)
   on_close=lambda ws: app.ws_close(ws))

@engn33r
Copy link
Collaborator

engn33r commented Feb 15, 2021

Commit 3112b7d removes and simplifies the callback behavior, effectively reverting it to the behavior seen in v0.48.0 before PR #442. I am closing this issue because the callback branch has been removed, but this callback code has a long history that I tried to outline in this comment. You are welcome to add a follow-up comment to commit 3112b7d if I missed something.

@engn33r engn33r closed this as completed Feb 15, 2021
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

8 participants