CaliDog / certstream-python

Python library for connecting to CertStream
MIT License
425 stars 72 forks source link

Instance parameter of on_error & on_open callbacks #31

Closed mcarpenter closed 3 years ago

mcarpenter commented 4 years ago

Hi,

There is a small bug in the on_open and on_error callbacks. The documentation for both indicates that the first parameter to the callback should be instance, which is intended to the instance of the originating certstream listener (which could be handy in the case where a consumer application has more than one listener). From https://github.com/CaliDog/certstream-python/blob/9e56f7e6586f8a94b4538edb632445ced7a4c3ec/README.md:

def on_open(instance):
    # Instance is the CertStreamClient instance that was opened
    print("Connection successfully established!")

def on_error(instance, exception):
    # Instance is the CertStreamClient instance that barfed
    print("Exception in CertStreamClient! -> {}".format(exception)) 

However, in that case the invocations of those handlers need to pass an explicit self parameter (else Python complains TypeError about the missing parameter and the handlers are not invoked). See:

Fix:

diff --git a/certstream/core.py b/certstream/core.py
index d590413..7c70990 100644
--- a/certstream/core.py
+++ b/certstream/core.py
@@ -30,7 +30,7 @@ class CertStreamClient(WebSocketApp):
     def _on_open(self):
         certstream_logger.info("Connection established to CertStream! Listening for events...")
         if self.on_open_handler:
-            self.on_open_handler()
+            self.on_open_handler(self)

     def _on_message(self, message):
         frame = json.loads(message)
@@ -44,7 +44,7 @@ class CertStreamClient(WebSocketApp):
         if type(ex) == KeyboardInterrupt:
             raise
         if self.on_error_handler:
-            self.on_error_handler(ex)
+            self.on_error_handler(self, ex)
         certstream_logger.error("Error connecting to CertStream - {} - Sleeping for a few seconds and trying again...".format(ex))

 def listen_for_events(message_callback, url, skip_heartbeats=True, setup_logger=True, on_open=None, on_error=None, **kwargs):

Example: this starts the listener, but the open callback fails with an error (and the error is eaten, erk):

import certstream
from certstream.cli import parser as certstream_cli

def callback(_instance):
    print('Startup callback')

URL = certstream_cli.get_default('url')
certstream.listen_for_events(callback, URL, on_open=callback)

If the _instance parameter is removed from callback() (or patch applied as above) then the open callback message is seen on stdout.

Fitblip commented 3 years ago

Hi there, I think this is fixed! Apologies for not closing it when it was.