Feature: Notification system #1766

relativisticelectron · 2022-06-18T12:58:26Z

This is a notification system for sync and async messages
Issue: #1695

Features:

A NotificationManager (1 global one) forwards notifications to UINotifications (such as LoggingNotifications, FlashNotifications, JSConsoleNotifications, JSNotifications, WebAPINotifications , which are instantiated for each user)
Notifications can be created from python and from javascript and handled both through NotificationManager in an identical way
Individual notifications can be sent to one or multiple UINotifications
Events such as onClose are possible for JSNotifications, WebAPINotifications
WebAPINotifications (= Notifications API ) is only available in localhost, tor-browser or https
If one UINotifications is not available, the notification is forwarded to a fallback UINotifications

Example call from javascript:

CreateNotification('this is the title', {target_uis:['js_message_box', 'WebAPI'], body:'body line 1\nline 2', image:'/static/img/ghost_3d.png', timeout:3000})

Further things that are enabled by this

Any "flash" messages could automatically also be logged, by changing

    def flash(self, *args, **kwargs):
       .....
        kwargs["target_uis"] = {"flash"}
       .....

(this could even be in some config file) to

    def flash(self, *args, **kwargs):
       .....
        kwargs["target_uis"] = {"flash", "logging"}
       .....

one can create python logging messages from javascript by calling

CreateNotification('this is a message', {target_uis:['logging'], body:'this is the body of the message'});

further callbacks, like onclick, onhover,.... are possible for JSNotifications, WebAPINotifications are straight forward to implement. One could even extend this to add Buttons in the Notifications.
more UINotifications (even if it is just a different design of JSNotifications, or you want to send a rpc command to another process, or start a program like notify-send) can be added very easily.

Limitations

WebAPINotifications (= Notifications API ) does not seem to support images in Linux, and Android for Firefox or Chromium.

Future extension ideas

persistent storage of messages
The open websocket connections could be used to allow a much more async rpc interaction. When sending an rpc call and the answer is not in time, the UI could be unfrozen and updated with a nice spinning wheel. This would allow the interaction with specter even though the rpc connection (perhaps because Bitcoin Core has to catch up to the tip) is extremely slow. The use case that I experienced: I "just" want to get a new receiving address and Bitcoin Core has lots of blocks to download. Specter is currently not usable during syncing.

Todos

** Current Status**

netlify · 2022-06-18T12:58:31Z

✅ Deploy Preview for specter-desktop-docs canceled.

Name	Link
🔨 Latest commit	`5aa0008`
🔍 Latest deploy log	https://app.netlify.com/sites/specter-desktop-docs/deploys/63d79fb38175590008956dd7

k9ert · 2022-06-19T10:21:54Z

This really looks awesome! I'll soon will look more into this. One thing i'd suggest right away:

app.specter.user_manager.get_user().notification_manager.flash(...

We should simplify this. Why not having somewhere a method like:

def flash(...)
    app.specter.user_manager.get_user().notification_manager.flash(...

in that case, we only need to change the import. Maybe we can put that method into src/cryptoadvance/specter/server_endpoints/controller.py ?

src/cryptoadvance/specter/node.py

relativisticelectron · 2022-06-19T12:04:53Z

This really looks awesome! I'll soon will look more into this.

Thanks. I am glad to hear this. Doing #778 with zmq will be straight forward once this PR is merged.

One thing i'd suggest right away:
app.specter.user_manager.get_user().notification_manager.flash(...
We should simplify this. Why not having somewhere a method like:
def flash(...)
    app.specter.user_manager.get_user().notification_manager.flash(...
in that case, we only need to change the import. Maybe we can put that method into src/cryptoadvance/specter/server_endpoints/controller.py ?

Yeah, this long command was annoying. I now put it in https://github.com/relativisticelectron/specter-desktop/blob/20220616_notification_system/src/cryptoadvance/specter/notifications/current_flask_user.py

k9ert

Here is a first feedback. I haven't looked at the javascript-code at all. Will do that later but maybe that might be the source of my misunderstandings.

src/cryptoadvance/specter/server_endpoints/wallets_api.py

src/cryptoadvance/specter/static/helpers.js

k9ert · 2022-06-19T10:58:49Z

src/cryptoadvance/specter/user.py

+ # setting up the notifications system
+ js_notifications = ui_notifications.JSNotifications()
+ webapi_notifications = ui_notifications.WebAPINotifications()
+ self.notification_manager = NotificationManager(


So ... every user gets its own NotificationManager? Can't we have a generic NotificationManager which is only there once? I know that e.g. the WalletManager is per User but i think that was an architectural error in the first place. So maybe we can have something similiar to the way encrypted Data is managed:
https://gist.github.com/k9ert/7cb1ca08453e848ae4ec03eb0fd0790a

A Generic Manager (in the diagram ServiceEncryptedStorageManager which is in this case derived from GenericDataManager) receives the call. It's only there once and initialized by the specter object (I don't like the singleton-logic in there due to flask way of handling the app #1634 ). It'll be Flask aware and derives the user either by current_user or by argument. It holds a cache of user-specific serviceEncryptedStorage objects (and creates them as needed) and simply hand over that call to the correct of those objects.

That way, the managers are all centrally available and not somehow obtained through obscure object-networks where you don't know whether the objects are there to manage other objects or rather carry data with minimalistic functionality. I'd like to have a clear distinction between "BusinessObjects" and managers. Managers are handling business-Objects but business-Objects should NOT reference Managers. Otherwise you'll get to a horrible situation and unclear responibilities and confusing initialisation/upgrading code, see e.g. #1661

Ok. I had the manager first instantiated in app.specter. However (of course) it was not differentiating between messages of different users. I would have to think about how and where to bring the user awareness in.

Ideally, this can be done at the endpoint-level. Making the managers non-user-aware is an advantage as then specter can be used independent of a flask-app. From an architectural point of view, that's a signifiant advantage.
However we already broke with that in the user_manager and the service_manager. In the service_manager, i mitigated that issue by assigning the object after the specter-initialisation. Similiar to how the specter-object is assigned to the Flask-App.
I think the mentioned ServiceEncryptedStorageManager is mitigating it through it's not so nice Singleton approach.
Maybe we can have a rule like: If it's Flask-aware, assign it to an attribute of Specter after initialisation. If it's not, let it be created in the __init__method of Specter class.
The UserManager is not mitigating that issue right now.

I do not understand all, but I experimented on a branch and moved the NotificationManager directly in the Specter class.

user independent UINotifications are created immediately

user aware UINotifications are created later (one could extend this to save/load in json, which UINotifications were present for the users)

Notifications and UINotifications have now a user_id to track to which user they belong to

See here the diff: https://github.com/relativisticelectron/specter-desktop/compare/20220616_notification_system...relativisticelectron:singleton?expand=1

Is that what you had in mind?

Passing in the NotificationManager imho doesn't improve the situation. Ideally businessObjects don't have many objects. They mostly carry data. Managers on the other hand, shouldn't carry much state and information but rather methods in order to manipulate businessObjects.
If you create a reference from a businessObject towards a manager, no matter whether you've initialized the manager in that businessObject or not, you're complicating things. E.g. testing: Instantiating a User is no longer that easy because you have to pass in a manager and if you pass some Mock of that Manager, you have to avoid using the User in a way which would trigger the Manager.
So, that's why i think that we should avoid making the businessObjects too clever and give them ways to be powerful. They should be passive and manipulated from the outside.

However (of course) it was not differentiating between messages of different users.

Well, there are two ways to achieve that:

You pass in the user for each method of the NotificationManager

You let the NotificationManager be aware of flask and let it import current_user

I wrote above that it would be cool from an architectural point of view to have managers NOT be flask aware if it's not needed. Because managers are usually instantiated within the specter-object and so, the specter-object gets flask aware as well and you can no longer use the specterobject without flask ( you get: "Outside of ApplicationContext blabla").

So in order to have Managers which are flask aware but still not make specter flask-aware, you can assign the manager after the initialisation of Specter like we do it with the ServiceManager:
https://github.com/relativisticelectron/specter-desktop/blob/master/src/cryptoadvance/specter/server.py#L148-L151

That way it's still comfortable for the calling code to get hold on a reference of that manager via app.specter. Hope that clarifies it? I would suggest you do it that way.

Thank you for all the feedback and explanation. I will work on it.

You pass in the user for each method of the NotificationManager

I did this now, and create the instance now after the Specter creation: https://github.com/relativisticelectron/specter-desktop/blob/e5de884350b8f338a50d23627ad1b8438184a5dd/src/cryptoadvance/specter/server.py#L154

src/cryptoadvance/specter/notifications/notification_manager.py

src/cryptoadvance/specter/notifications/notifications.py

and cryptoadvance#1766 (comment)

see cryptoadvance#1766 (comment)

k9ert · 2022-07-01T09:24:23Z

I'll try to do another round of review the coming days. However i already know what i'm still concerned about: The constant requests every 5 seconds. How easy/difficult would it be to replace that by websockets?
If it's not part of this PR, maybe we can already create another PR (based on this one) where we put in websockets so that we can be pretty sure to make a release which contains both PRs?

Update:
Just realized that there is also some documentation of how this works here:
#1695 (comment)

relativisticelectron · 2022-07-01T10:47:24Z

I'll try to do another round of review the coming days. However i already know what i'm still concerned about: The constant requests every 5 seconds. How easy/difficult would it be to replace that by websockets? If it's not part of this PR, maybe we can already create another PR (based on this one) where we put in websockets so that we can be pretty sure to make a release which contains both PRs?

Yes, websockets seem to be the solution for the repeated 5s requests. And they seem to be relatively straight forward too: https://www.alanwsmith.com/posts/setup-a-python-flask-web-server-with-websockets--20eo3p4t928f

I can develop the websocket in a websocket branch (branching off this PR).

EDIT: websockets is kind of fun. But I have to figure out how to run it properly besides flask and send messages between the main thread and the websockts-server thread.

k9ert · 2022-07-04T09:58:59Z

EDIT: websockets is kind of fun. But I have
to figure out how to run it properly besides flask and send messages between the main thread and the websockts-server thread.

I'm currently also diving into sockets (without web-) and it's not very convenient coming from the rest-world. However in your case i don't think it's very complicated. You only need to communicate in one direction (from the server to the browser) and there is almost (at least in the beginning) only one message: "There are news! Reload!".

relativisticelectron · 2022-07-04T11:09:04Z

I'm currently also diving into sockets (without web-) and it's not very convenient coming from the rest-world. However in your case i don't think it's very complicated. You only need to communicate in one direction (from the server to the browser) and there is almost (at least in the beginning) only one message: "There are news! Reload!".

Yeah, the requests are way easier to implement. Currently I am thinking about how to get a message from python (like a notification) into the infinite websockets server loop (which then can send it to the browser):

    async def handler(self, websocket, path):  # don't remove path
        await self.register(websocket)
        try:
            async for message in websocket:  # this is an endless loop waiting for incoming websocket messages
                msg = await self.send_messages_to_all_connected_websockets(
                    message, exclude_websockets={websocket}
                )
                await websocket.send(msg)
        finally:
            await self.unregister(websocket)

One way is to create a python websocket client and connect to the server (link). This allows sending messages into the infinite loop. Do you know a better way?

EDIT: My idea really seems to be a legit way to do it..... https://stackoverflow.com/questions/54648015/python-websockets-how-to-send-message-from-function ... not elegant... buy I can do it that way.

k9ert · 2022-07-04T14:46:31Z

I'm not sure i've understood the problem correctly. I took the example from:
https://www.alanwsmith.com/posts/setup-a-python-flask-web-server-with-websockets--20eo3p4t928f
... and modified it so that you can send msgs via http:
https://gist.github.com/k9ert/9f91bfe94e9942ea0e02733e958fc26d

Shouldn't that do it? I mean this should also be possible in a server-side loop instead of a flask-endpoint, no?

relativisticelectron · 2022-07-04T15:44:29Z

I'm not sure i've understood the problem correctly. I took the example from: https://www.alanwsmith.com/posts/setup-a-python-flask-web-server-with-websockets--20eo3p4t928f ... and modified it so that you can send msgs via http: https://gist.github.com/k9ert/9f91bfe94e9942ea0e02733e958fc26d

Shouldn't that do it? I mean this should also be possible in a server-side loop instead of a flask-endpoint, no?

Yes, that is the same logic as my code. I find it however not elegant, that a "long lived" connection/protocol, needs a fresh (python) client to (python) server websockets.connect connection every time I want to broadcast from the (python) server to the browser.

The newest version has now this connection be also forever, and it works :-) https://github.com/relativisticelectron/specter-desktop/blob/20220703_websockets/src/cryptoadvance/specter/websockets_server.py#L142

EDIT: But the code is much more complex then the simple javascript polling that is currently in the PR.... And I put no consideration (yet) in for blocked ports, port config file, dropped connection (and trying to reconnect), user-awareness, ....

@k9ert : What do you think?

Is the code complexity worth it getting rid of the 5 second polling? We could also set it to 20s polling. For transaction notification that would be sufficient. And if we think the user needs more "instant" messages we could extend it to websockets.

k9ert · 2022-07-04T19:47:56Z

Is the code complexity worth it getting rid of the 5 second polling?

I don't see that many code complexity. Sure, it's more complex than the polling but the effort of opening http-connections every x seconds and polluting the log is imho worth that complexity.
Not sure what you mean with blocked ports. Port config is easy, we already have a config-system. Dropped connection might be the biggest issue which needs to get addressed but imho it's worth the risk of not delivering messages on time.
What do you mean by user-awareness?

relativisticelectron · 2022-07-05T05:51:44Z

I don't see that many code complexity. Sure, it's more complex than the polling but the effort of opening http-connections every x seconds and polluting the log is imho worth that complexity.

ok.

What do you mean by user-awareness?

I have to figure out how the websocket message goes only to the intended logged-in user, and not to all users (also the server has to check that answers come from the right user). In the current PR (polling) that is done via flask_login.current_user (for sending and receiving messages). See here: https://stackoverflow.com/questions/20863629/flask-login-flask-sockets-chaos

Idea: When loading the page, javascript requests a random user token from the server. Then this user token can be passed directly after opening the socket connection. Then the Websocket-Server can associate the open websocket connection with the user token. --> Done

k9ert · 2022-12-14T12:35:56Z

src/cryptoadvance/specter/config.py

@@ -57,6 +57,7 @@ class BaseConfig(object):
 # The prefix for extensions which don't get access to the session cookie (if SPECTER_URL_PREFIX isn't compromised)
 ISOLATED_CLIENT_EXT_URL_PREFIX = "/ext"

+ HOST = os.getenv("HOST", "127.0.0.1")


Can't we move that to the config of the extension?

I need the app.config["HOST"] in the notification extension. Adding it to the global BaseConfig, does make it consistent with PORT. And if another HOST value if given for the server then the config value is updated here https://github.com/relativisticelectron/specter-desktop/blob/bf03c3236958c49405b7d56c71e0b6ab67c9f5e4/src/cryptoadvance/specter/cli/cli_server.py#L127

I don't really see how to move this to the service.

k9ert · 2022-12-14T12:45:50Z

src/cryptoadvance/specter/templates/includes/helpers.jinja

+
+
+ async function sendRequest(url, method_str, formData) {
+ if (!formData) {
+ formData = new FormData();
+ }
+ formData.append("csrf_token", "{{ csrf_token() }}")
+ d = {
+ method: method_str,
+ }
+ if (method_str == 'POST') {
+ d['body'] = formData;
+ }
+
+ const response = await fetch(url, d);
+ if(response.status != 200){
+ showError(await response.text());
+ console.log(`Error while calling ${url} with ${method_str} ${formData}`)
+ return
+ }
+ return await response.json();
+ }
+
+
+


This function is, in the meantime, already implemented in static/helpers.js Can you remove this here and use the other version ?

Done in 0d69ae8

k9ert · 2022-12-14T13:04:05Z

src/cryptoadvance/specter/templates/base.jinja

@@ -172,6 +172,7 @@
 {% endif %}
 </script>
 {% endif %}
+ <!-- This scripts block will be overwritten in /device -->


I'm not sure i understand the comment. The blocks are there to be overwritten. Sure, some of them indeed are. But why do we comment that here?

That was a reminder for me, that {% include "includes/language/language_js.jinja" %} is actually not imported any more in /device (If I remember correctly). I am actually not sure if this behavior was intentional. I can remove the comment.

k9ert · 2022-12-14T14:20:22Z

src/cryptoadvance/specter/specter.py

+ # a list of functions that are called at cleanup_on_exit taking in each signum, frame
+ self.call_functions_at_cleanup_on_exit = []
+
+ def service_manager_cleanup_on_exit(signum, frame):
+ return self.service_manager.execute_ext_callbacks(
+ callbacks.cleanup_on_exit, signum, frame
+ )
+
+ self.call_functions_at_cleanup_on_exit.append(service_manager_cleanup_on_exit)


This is IMHO problematic as it's making the specter-object more aware of the serviceManager. We currently only pass ot down to other managers.
Imho a reasonable solution would be to create the clean_on_exit method to server.py and move the registering to server.py. Then we could call the specter.clean_on_exit function from there OR, even better, register both functions.
This second option is currently prevented by this this:

# For some reason we need to explicitely exit here. Otherwise it will hang exit(0)

Is this currently the case? I can't reproduce it but i know that this was, at some point, necessary for Stepan and maybe even @moneymanolis on Mac? Can we fix that somehow?

Can't recall from the top of my head.

This is IMHO problematic as it's making the specter-object more aware of the serviceManager.

Agreed. Done in bac4422

- removed duplicate sendRequest

see cryptoadvance#1766 (comment)

k9ert · 2023-01-10T21:03:49Z

Shutting down specter, i get this:

[2023-01-10 21:52:27,440] INFO in wallet_manager:   * loaded_wallets: 4
^C[2023-01-10 22:00:32,115] INFO in internal_node: STOPPING bitcoind Specter Bitcoin from status Running
[2023-01-10 22:00:32,115] INFO in internal_node: STOPPING bitcoind Specter Bitcoin from status Running
[2023-01-10 22:00:32,116] INFO in node_controller: Cleaning up (cleanup_hard:False , datadir:/home/kim/.specter_dev/nodes/specter_bitcoin/.bitcoin-main)
[2023-01-10 22:00:32,116] INFO in node_controller: Cleaning up (cleanup_hard:False , datadir:/home/kim/.specter_dev/nodes/specter_bitcoin/.bitcoin-main)
[2023-01-10 22:00:32,116] INFO in node_controller: Terminated bitcoind with pid 2423152, waiting for termination (timeout 50 secs)...
[2023-01-10 22:00:32,116] INFO in node_controller: Terminated bitcoind with pid 2423152, waiting for termination (timeout 50 secs)...
[2023-01-10 22:00:32,123] INFO in internal_node: bitcoind Specter Bitcoin stopped
[2023-01-10 22:00:32,123] INFO in internal_node: STOPPING process complete Specter Bitcoin to status Down
[2023-01-10 22:00:32,123] DEBUG in service: callback_cleanup_on_exit called of NotificationsService
[2023-01-10 22:00:32,123] DEBUG in notification_manager: NotificationManager quit was called.
[2023-01-10 22:00:32,123] DEBUG in websockets_server_client: quit_server was called.
[2023-01-10 22:00:32,123] INFO in specter: Closing Specter after cleanup
[2023-01-10 22:00:32,123] DEBUG in websockets_server_client: Unregistered <simple_websocket.ws.Server object at 0x7ff420389690> belonging to Python broadcaster Client, started at 2023-01-10 20:10:20.968541
[2023-01-10 22:00:32,124] INFO in _internal: 127.0.0.1 - - [10/Jan/2023 22:00:32] "GET /svc/notifications/websocket HTTP/1.1" 200 -
[2023-01-10 22:00:32,124] WARNING in specter: Failed to connect to Tor control port. Error: [Errno 111] Connection refused
[2023-01-10 22:00:32,125] ERROR in cli_server: Could not initialize tor-system
[2023-01-10 22:00:32,125] INFO in node_controller: Cleaning up (cleanup_hard:False , datadir:/home/kim/.specter_dev/nodes/specter_bitcoin/.bitcoin-main)
[2023-01-10 22:00:32,125] INFO in node_controller: Terminated bitcoind with pid 2423152, waiting for termination (timeout 50 secs)...
[2023-01-10 22:00:37,900] INFO in internal_node: bitcoind Specter Bitcoin stopped
[2023-01-10 22:00:37,900] INFO in internal_node: STOPPING process complete Specter Bitcoin to status Down
[2023-01-10 22:00:37,900] DEBUG in service: callback_cleanup_on_exit called of NotificationsService
[2023-01-10 22:00:37,900] WARNING in specter: Failed to connect to Tor control port. Error: [Errno 111] Connection refused
[2023-01-10 22:00:37,900] ERROR in cli_server: Could not initialize tor-system
Traceback (most recent call last):
  File "/home/kim/.pyenv/versions/3.10.4/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/kim/.pyenv/versions/3.10.4/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/__main__.py", line 7, in <module>
    entry_point()
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/cli/cli_server.py", line 223, in server
    run(debug=debug)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/cli/cli_server.py", line 210, in run
    app.run(debug=debug, **kwargs)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 920, in run
    run_simple(t.cast(str, host), port, self, **options)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/werkzeug/serving.py", line 1009, in run_simple
    _rwr(
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/werkzeug/_reloader.py", line 428, in run_with_reloader
    sys.exit(reloader.restart_with_reloader())
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/werkzeug/_reloader.py", line 248, in restart_with_reloader
    exit_code = subprocess.call(args, env=new_environ, close_fds=False)
  File "/home/kim/.pyenv/versions/3.10.4/lib/python3.10/subprocess.py", line 347, in call
    return p.wait(timeout=timeout)
  File "/home/kim/.pyenv/versions/3.10.4/lib/python3.10/subprocess.py", line 1204, in wait
    return self._wait(timeout=timeout)
  File "/home/kim/.pyenv/versions/3.10.4/lib/python3.10/subprocess.py", line 1938, in _wait
    (pid, sts) = self._try_wait(0)
  File "/home/kim/.pyenv/versions/3.10.4/lib/python3.10/subprocess.py", line 1896, in _try_wait
    (pid, sts) = os.waitpid(self.pid, wait_flags)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/specter.py", line 216, in cleanup_on_exit
    f(signum, frame)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/specter.py", line 200, in service_manager_cleanup_on_exit
    return self.service_manager.execute_ext_callbacks(
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/managers/service_manager/service_manager.py", line 297, in execute_ext_callbacks
    raise e
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/managers/service_manager/service_manager.py", line 289, in execute_ext_callbacks
    return_values[ext.id] = getattr(ext, f"callback_{callback_id}")(
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/service.py", line 84, in callback_cleanup_on_exit
    self.notification_manager.quit()
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/notification_manager.py", line 138, in quit
    self.websockets_client.quit_server()
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/websockets_server_client.py", line 432, in quit_server
    self.send(message_dictionary)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/websockets_server_client.py", line 392, in send
    self.websocket.send(robust_json_dumps(message_dictionary))
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/simple_websocket/ws.py", line 92, in send
    raise ConnectionClosed(self.close_reason, self.close_message)
simple_websocket.ws.ConnectionClosed: Connection closed: 1005

Any idea what might caught that? Can you fix that before the merge? Apart from that, it's imho ready to merge. I've tested it via the dev-extension and added a video in the above description.

relativisticelectron · 2023-01-11T16:25:20Z

Shutting down specter, i get this:

I cannot reproduce this error; I am using CTRL-C to close the specter server. How do you shut down specter (gracefully)?

However it is clear why the error occurs

callback_cleanup_on_exit called of NotificationsService

is called twice, and then it tries to send a websocket message to a shutdown websocket server.

EDIT: Can you check if 7040326 solved it?

k9ert · 2023-01-17T14:27:21Z

That commit seem to have made it worse. The websocket-connection is somehow breaking. Here is a curated Log:

[2023-01-17 15:21:31,738] DEBUG in service_manager:     setting SPECTER_NOTIFICATIONS_WEBSOCKETS_ENABLED = True
[...]
[2023-01-17 15:21:33,246] INFO in server: ----> starting service callback_after_serverpy_init_app 
[2023-01-17 15:21:33,246] DEBUG in notification_manager: Registering "logging" for user "None" in NotificationManager
[2023-01-17 15:21:33,246] DEBUG in notification_manager: Registering "print" for user "None" in NotificationManager
[2023-01-17 15:21:33,246] DEBUG in notification_manager: Registering "flash" for user "None" in NotificationManager
[2023-01-17 15:21:33,246] INFO in websockets_server_client: Create WebsocketServer
[2023-01-17 15:21:33,246] DEBUG in websockets_server_client: set_as_broadcaster ...U7vaw
[2023-01-17 15:21:33,247] INFO in websockets_server_client: Connecting WebsocketClient
[2023-01-17 15:21:33,247] DEBUG in notification_manager: Registering "webapi" for user "admin" in NotificationManager
[2023-01-17 15:21:33,247] DEBUG in notification_manager: Registering "js_message_box" for user "admin" in NotificationManager
[2023-01-17 15:21:33,247] DEBUG in notification_manager: Registering "js_console" for user "admin" in NotificationManager
[2023-01-17 15:21:33,248] DEBUG in notification_manager: Registering "webapi" for user "kim" in NotificationManager
[2023-01-17 15:21:33,248] DEBUG in notification_manager: Registering "js_message_box" for user "kim" in NotificationManager
[2023-01-17 15:21:33,248] DEBUG in notification_manager: Registering "js_console" for user "kim" in NotificationManager
[2023-01-17 15:21:33,252] WARNING in _internal:  * Debugger is active!
[2023-01-17 15:21:33,255] INFO in _internal:  * Debugger PIN: 140-692-187
[2023-01-17 15:21:33,605] DEBUG in controller: websocket route called. This will start a new websocket connection.
[2023-01-17 15:21:33,606] DEBUG in websockets_server_client: Created WebsocketClient connection to url ws://127.0.0.1:25441/svc/notifications/websocket
[2023-01-17 15:21:33,607] DEBUG in controller: websocket route called. This will start a new websocket connection.
[2023-01-17 15:21:33,608] INFO in websockets_server_client: Started websocket connection <simple_websocket.ws.Server object at 0x7f2f1c134550> between the server and a new client
[2023-01-17 15:21:33,608] DEBUG in websockets_server_client: Created WebsocketClient connection to url ws://127.0.0.1:25441/svc/notifications/websocket
[2023-01-17 15:21:33,608] WARNING in websockets_server_client: user_token ...Nt0Xs not found in users
[2023-01-17 15:21:33,608] INFO in websockets_server_client: Started websocket connection <simple_websocket.ws.Server object at 0x7f2f1c135270> between the server and a new client
[2023-01-17 15:21:33,611] INFO in websockets_server_client: python-websocket-client --> python-websocket-server was first used and registered.
[2023-01-17 15:21:34,040] DEBUG in wallet_manager: starting update of wallet_manager
[2023-01-17 15:21:34,042] INFO in wallet_manager: Iterating over 6 wallet files in /home/kim/.specter_dev/wallets/main
[2023-01-17 15:21:34,043] INFO in wallet_manager: Using threads in updating the wallet manager.
[2023-01-17 15:21:34,043] DEBUG in flask: starting new FlaskThread: _update
[2023-01-17 15:21:34,043] INFO in wallet_manager: Started updating wallets with 6 wallets
[...]
023-01-17 15:21:38,434] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /svc/notifications/get_websockets_info/ HTTP/1.1" 200 -
[2023-01-17 15:21:38,476] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /static/styles.css HTTP/1.1" 304 -
[2023-01-17 15:21:38,482] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "POST /wallets/wallets_overview/txlist HTTP/1.1" 200 -
[2023-01-17 15:21:38,503] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /static/img/down-arrow.svg HTTP/1.1" 304 -
[2023-01-17 15:21:38,896] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /static/img/send.svg HTTP/1.1" 304 -
[2023-01-17 15:21:38,901] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /static/img/transfer.svg HTTP/1.1" 304 -
[2023-01-17 15:21:38,956] ERROR in controller: Uncaught exception: 400 Bad Request: The browser (or proxy) sent a request that this server could not understand.
[2023-01-17 15:21:38,957] ERROR in controller: Traceback (most recent call last):
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1523, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1499, in dispatch_request
    self.raise_routing_exception(req)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1481, in raise_routing_exception
    raise request.routing_exception  # type: ignore
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/ctx.py", line 397, in match_request
    result = self.url_adapter.match(return_rule=True)  # type: ignore
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/werkzeug/routing.py", line 2026, in match
    raise WebsocketMismatch()
werkzeug.routing.WebsocketMismatch: 400 Bad Request: The browser (or proxy) sent a request that this server could not understand.

[2023-01-17 15:21:38,965] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /svc/notifications/websocket HTTP/1.1" 500 -
[2023-01-17 15:21:38,965] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:38] "GET /static/img/receive.svg HTTP/1.1" 304 -
[2023-01-17 15:21:48,987] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:48] "GET /svc/notifications/get_websockets_info/ HTTP/1.1" 200 -
[2023-01-17 15:21:59,010] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:21:59] "GET /svc/notifications/get_websockets_info/ HTTP/1.1" 200 -
[2023-01-17 15:22:09,028] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:22:09] "GET /svc/notifications/get_websockets_info/ HTTP/1.1" 200 -
[2023-01-17 15:22:09,037] ERROR in controller: Uncaught exception: 400 Bad Request: The browser (or proxy) sent a request that this server could not understand.
[2023-01-17 15:22:09,038] ERROR in controller: Traceback (most recent call last):
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1523, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1499, in dispatch_request
    self.raise_routing_exception(req)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1481, in raise_routing_exception
    raise request.routing_exception  # type: ignore
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/ctx.py", line 397, in match_request
    result = self.url_adapter.match(return_rule=True)  # type: ignore
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/werkzeug/routing.py", line 2026, in match
    raise WebsocketMismatch()
werkzeug.routing.WebsocketMismatch: 400 Bad Request: The browser (or proxy) sent a request that this server could not understand.

[2023-01-17 15:22:09,039] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:22:09] "GET /svc/notifications/websocket HTTP/1.1" 500 -
[2023-01-17 15:22:19,049] INFO in _internal: 127.0.0.1 - - [17/Jan/2023 15:22:19] "GET /svc/notifications/get_websockets_info/ HTTP/1.1" 200 -

relativisticelectron

Did you do pip3 install -r requirements.txt --require-hashes? (Line 2026 in .env/lib/python3.10/site-packages/werkzeug/routing.py looks different for me)

How can I reproduce this error?

k9ert · 2023-01-18T09:36:48Z

My bad! That issue disappeared but the ConnectionClosed did not. I assume that this is because of the DevelopmentConfig. Could only reproduce it there and it makes sense as Flask has some weird double-app-start issue there.
So for me, something like this did the job. Not very elegant but acceptable:

    def callback_cleanup_on_exit(self, signum=0, frame=0):
        logger.debug(f"callback_cleanup_on_exit called of {self.__class__.__name__}")
        try:
            self.notification_manager.quit()
        except ConnectionClosed as cc:
            if app.config["SPECTER_CONFIGURATION_CLASS_FULLNAME"].endswith(
                "DevelopmentConfig"
            ):
                logger.debug(f"Because in DevelopmentConfig, Exception swallowed while closing connection: {cc}")
            else:
                raise cc

But maybe you want that idiom to be implemented somewhere deeper in the stack?!

k9ert · 2023-01-18T09:49:52Z

I've also resolved the conflict in requirement.txt and there were those changes:

exceptiongroup==1.0.4 --> 1.0.0
pytz==2022.6 --> 2022.5
And removed backports.zoneinfo as we're no longer supporting python < 3.10 in development (at least without manually installing that package).

relativisticelectron · 2023-01-18T16:00:06Z

My bad! That issue disappeared but the ConnectionClosed did not. I assume that this is because of the DevelopmentConfig. Could only reproduce it there and it makes sense as Flask has some weird double-app-start issue there. So for me, something like this did the job. Not very elegant but acceptable:
    def callback_cleanup_on_exit(self, signum=0, frame=0):
        logger.debug(f"callback_cleanup_on_exit called of {self.__class__.__name__}")
        try:
            self.notification_manager.quit()
        except ConnectionClosed as cc:
            if app.config["SPECTER_CONFIGURATION_CLASS_FULLNAME"].endswith(
                "DevelopmentConfig"
            ):
                logger.debug(f"Because in DevelopmentConfig, Exception swallowed while closing connection: {cc}")
            else:
                raise cc
But maybe you want that idiom to be implemented somewhere deeper in the stack?!

I would prefer to solve the problem where it occurs. How can I reproduce the error?

k9ert · 2023-01-18T20:22:48Z

I'm not aware doing anything special. python 3.10.4. Starting like below. CTRL-C for stopping the process.

(.env) kim@nucii➜  specter-desktop git:(20220616_notification_system) ✗ python3 -m cryptoadvance.specter server --config DevelopmentConfig --debug
[2023-01-18 21:19:02,767] DEBUG in cli_server: We're now on level DEBUG on logger cryptoadvance
[2023-01-18 21:19:02,768] INFO in server: Configuration: cryptoadvance.specter.config.DevelopmentConfig
[2023-01-18 21:19:02,768] INFO in server: SPECTER_DATA_FOLDER: /home/kim/.specter_dev
[2023-01-18 21:19:02,771] DEBUG in genericdata_manager: Loading existing file /home/kim/.specter_dev/migration_data.json
[2023-01-18 21:19:02,771] INFO in specter_migrator: Initiated MigDataManager(/home/kim/.specter_dev/migration_data.json events:73 execs:3 )
[2023-01-18 21:19:02,771] DEBUG in specter_migrator: Initiated SpecterMigrator(MigDataManager(/home/kim/.specter_dev/migration_data.json events:73 execs:3 ))
[2023-01-18 21:19:02,772] INFO in specter_migrator: Collecting possible migrations ...
[2023-01-18 21:19:02,772] INFO in specter_migrator: Collecting possible migrations ...
[2023-01-18 21:19:02,772] INFO in specter_migrator: Collecting possible migrations ...
[...] # see the TRL-C below
[2023-01-18 21:19:07,225] INFO in wallet_manager:   * loaded_wallets: 4
^C[2023-01-18 21:19:10,918] INFO in internal_node: STOPPING bitcoind Specter Bitcoin from status Running
[2023-01-18 21:19:10,918] INFO in internal_node: STOPPING bitcoind Specter Bitcoin from status Running
[2023-01-18 21:19:10,918] INFO in node_controller: Cleaning up (cleanup_hard:False , datadir:/home/kim/.specter_dev/nodes/specter_bitcoin/.bitcoin-main)
[2023-01-18 21:19:10,918] INFO in node_controller: Cleaning up (cleanup_hard:False , datadir:/home/kim/.specter_dev/nodes/specter_bitcoin/.bitcoin-main)
[2023-01-18 21:19:10,918] INFO in node_controller: Terminated bitcoind with pid 3147573, waiting for termination (timeout 50 secs)...
[2023-01-18 21:19:10,918] INFO in node_controller: Terminated bitcoind with pid 3147573, waiting for termination (timeout 50 secs)...
[...]
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/service.py", line 84, in callback_cleanup_on_exit
    self.notification_manager.quit()
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/notification_manager.py", line 138, in quit
    self.websockets_client.quit_server()
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/websockets_server_client.py", line 439, in quit_server
    self.send(message_dictionary)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specterext/notifications/websockets_server_client.py", line 392, in send
    self.websocket.send(robust_json_dumps(message_dictionary))
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/simple_websocket/ws.py", line 92, in send
    raise ConnectionClosed(self.close_reason, self.close_message)
simple_websocket.ws.ConnectionClosed: Connection closed: 1005 
[2023-01-18 21:19:11,151] INFO in node_controller: Cleaning up (cleanup_hard:False , datadir:/home/kim/.specter_dev/nodes/specter_bitcoin/.bitcoin-main)
[2023-01-18 21:19:11,151] INFO in node_controller: Terminated bitcoind with pid 3147573, waiting for termination (timeout 50 secs)...
(.env) kim@nucii ➜  specter-desktop git:(20220616_notification_system) ✗ python3 --version                                                         
Python 3.10.4
(.env) kim@nucii➜  specter-desktop git:(20220616_notification_system) ✗

relativisticelectron · 2023-01-19T17:50:03Z

The merge of master 4cec1f4 causes for me a loss of the saved node information. Same if I switch to current master.
Edited: ~~(That is likely also responsible for the failing test) See #2070~~

…ification_system # Conflicts: # requirements.txt # tests/test_managers_service.py # tests/test_util_reflection.py

k9ert · 2023-01-30T10:51:18Z

I'm ready to merge this! Any last words?

k9ert · 2023-01-31T08:31:35Z

The longest running PR is finally merged. Took 227 days, 250 commits, 149 comments.
Thank you @relativisticelectron.

A feature for a notification system has been added, including websockets and SSL support, improved error handling, and better logging. There have been various bug fixes, refactoring, and documentation updates throughout the process. The feature is now complete and includes new functionality for managing and displaying notifications, as well as testing and debugging tools.m>

A feature for a notification system has been added, including websockets and SSL support, improved error handling, and better logging. There have been various bug fixes, refactoring, and documentation updates throughout the process. The feature is now complete and includes new functionality for managing and displaying notifications, as well as testing and debugging tools.m> Co-authored-by: relativisticelectron <[email protected]>

relativisticelectron marked this pull request as draft June 18, 2022 13:41

k9ert reviewed Jun 19, 2022

View reviewed changes

src/cryptoadvance/specter/node.py Outdated Show resolved Hide resolved

k9ert reviewed Jun 19, 2022

View reviewed changes

k9ert mentioned this pull request Jun 19, 2022

fix starting server with hardware wallet plugged in #1748

Closed

relativisticelectron added a commit to relativisticelectron/specter-desktop that referenced this pull request Jun 19, 2022

did cryptoadvance#1766 (comment)

423ba71

relativisticelectron added a commit to relativisticelectron/specter-desktop that referenced this pull request Jun 19, 2022

did cryptoadvance#1766 (comment)

8ab356f

relativisticelectron added a commit to relativisticelectron/specter-desktop that referenced this pull request Jun 19, 2022

addressed cryptoadvance#1766 (comment)

70efd37

and cryptoadvance#1766 (comment)

relativisticelectron added a commit to relativisticelectron/specter-desktop that referenced this pull request Jun 19, 2022

renaming because it was confusing

6b72b24

see cryptoadvance#1766 (comment)

relativisticelectron added a commit to relativisticelectron/specter-desktop that referenced this pull request Jun 20, 2022

refactoring and cryptoadvance#1766 (comment)

2819986

relativisticelectron added a commit to relativisticelectron/specter-desktop that referenced this pull request Jun 20, 2022

did cryptoadvance#1766 (comment)

68d9640

relativisticelectron marked this pull request as ready for review June 22, 2022 17:23

relativisticelectron marked this pull request as draft June 22, 2022 17:51

relativisticelectron marked this pull request as ready for review June 22, 2022 17:54

relativisticelectron marked this pull request as draft June 23, 2022 10:42

relativisticelectron marked this pull request as ready for review June 23, 2022 11:27

relativisticelectron added 4 commits July 6, 2022 14:03

added websocket reconnect javascript

87da0d7

timeout fix

3e740d3

FIXED server timout issue

77752d8

working port config

5de1e35

k9ert reviewed Dec 14, 2022

View reviewed changes

k9ert mentioned this pull request Dec 14, 2022

Chore: Preparation for Notification System #2017

Closed

Merge branch 'master' into 20220616_notification_system

bf03c32

k9ert reviewed Dec 14, 2022

View reviewed changes

relativisticelectron added 3 commits December 15, 2022 08:58

done cryptoadvance#1766 (comment)

0d69ae8

- removed duplicate sendRequest

moved service_manager_cleanup_on_exit

bac4422

see cryptoadvance#1766 (comment)

added comment

e7dff40

see cryptoadvance#1766 (comment)

attempt to fix cryptoadvance#1766 (comment)

7040326

relativisticelectron commented Jan 17, 2023

View reviewed changes

Merge branch 'master' into 20220616_notification_system

4cec1f4

Merge branch 'master' into 20220616_notification_system

1f05655

relativisticelectron and others added 3 commits January 28, 2023 21:04

Merge remote-tracking branch 'cryptoadvance/master' into 20220616_not…

a75d44f

…ification_system # Conflicts: # requirements.txt # tests/test_managers_service.py # tests/test_util_reflection.py

pytest works. Was a variation in requirements.txt

d77efed

Merge branch 'master' into 20220616_notification_system

5aa0008

k9ert approved these changes Jan 30, 2023

View reviewed changes

k9ert merged commit 8eef5c7 into cryptoadvance:master Jan 31, 2023

Feature: Notification system #1766

Feature: Notification system #1766

Conversation

relativisticelectron commented Jun 18, 2022 • edited by k9ert Loading

netlify bot commented Jun 18, 2022 • edited Loading

✅ Deploy Preview for specter-desktop-docs canceled.

k9ert commented Jun 19, 2022

relativisticelectron commented Jun 19, 2022 • edited Loading

k9ert left a comment

Choose a reason for hiding this comment

k9ert Jun 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

relativisticelectron Jun 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k9ert commented Jul 1, 2022 • edited Loading

relativisticelectron commented Jul 1, 2022 • edited Loading

k9ert commented Jul 4, 2022

relativisticelectron commented Jul 4, 2022 • edited Loading

k9ert commented Jul 4, 2022

relativisticelectron commented Jul 4, 2022 • edited Loading

k9ert commented Jul 4, 2022

relativisticelectron commented Jul 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

relativisticelectron Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

relativisticelectron Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

k9ert commented Jan 10, 2023 • edited Loading

relativisticelectron commented Jan 11, 2023 • edited Loading

k9ert commented Jan 17, 2023

relativisticelectron left a comment • edited Loading

Choose a reason for hiding this comment

k9ert commented Jan 18, 2023

k9ert commented Jan 18, 2023

relativisticelectron commented Jan 18, 2023 • edited Loading

k9ert commented Jan 18, 2023

relativisticelectron commented Jan 19, 2023 • edited Loading

k9ert commented Jan 30, 2023

k9ert commented Jan 31, 2023

relativisticelectron commented Jun 18, 2022 •

edited by k9ert

Loading

netlify bot commented Jun 18, 2022 •

edited

Loading

relativisticelectron commented Jun 19, 2022 •

edited

Loading

k9ert Jun 19, 2022 •

edited

Loading

relativisticelectron Jun 20, 2022 •

edited

Loading

k9ert commented Jul 1, 2022 •

edited

Loading

relativisticelectron commented Jul 1, 2022 •

edited

Loading

relativisticelectron commented Jul 4, 2022 •

edited

Loading

relativisticelectron commented Jul 4, 2022 •

edited

Loading

relativisticelectron commented Jul 5, 2022 •

edited

Loading

relativisticelectron Dec 15, 2022 •

edited

Loading

relativisticelectron Dec 15, 2022 •

edited

Loading

k9ert commented Jan 10, 2023 •

edited

Loading

relativisticelectron commented Jan 11, 2023 •

edited

Loading

relativisticelectron left a comment •

edited

Loading

relativisticelectron commented Jan 18, 2023 •

edited

Loading

relativisticelectron commented Jan 19, 2023 •

edited

Loading