openwisp / openwisp-notifications

Notifications module of OpenWISP
https://openwisp.io/docs/dev/notifications/
GNU General Public License v3.0
41 stars 42 forks source link

[bug] notifications JS widget fails in admin login page #70

Closed nemesifier closed 4 years ago

nemesifier commented 4 years ago

How to replicate:

Expected result:

Actual result:

reconnecting-websocket.min.js:3 WebSocket connection to 'ws://localhost:8000/ws/notifications/' failed: Error during WebSocket handshake: Unexpected response code: 403
open @ reconnecting-websocket.min.js:3
a @ reconnecting-websocket.min.js:3
(anonymous) @ notifications.js:4
jquery.js:4046 jQuery.Deferred exception: Cannot read property 'scrollHeight' of undefined TypeError: Cannot read property 'scrollHeight' of undefined
    at onUpdate (http://localhost:8000/static/openwisp_notifications/js/notifications.js:124:58)
    at initNotificationWidget (http://localhost:8000/static/openwisp_notifications/js/notifications.js:165:9)
    at notificationWidget (http://localhost:8000/static/openwisp_notifications/js/notifications.js:177:5)
    at HTMLDocument.<anonymous> (http://localhost:8000/static/openwisp_notifications/js/notifications.js:14:9)
    at mightThrow (http://localhost:8000/static/admin/js/vendor/jquery/jquery.js:3762:29)
    at process (http://localhost:8000/static/admin/js/vendor/jquery/jquery.js:3830:12) undefinednnection to 'ws://localhost:8000/ws/notifications/' failed: Error during WebSocket handshake: Unexpected response code: 403
open @ reconnecting-websocket.min.js:3
(anonymous) @ reconnecting-websocket.min.js:3
pandafy commented 4 years ago

This can be easily fixed by setting debug option as described in the used library's documentation.

In my opinion it will be better to use pladaria/reconnecting-websocket instead of current one, since it is presumably more updated(maintained). I should have checked that before choosing the current library. :sweat_smile: . It also provides an option to turn off debug messages.

One more option which can be added is to disable re-connection conditionally (if client receives 403 response).

nemesifier commented 4 years ago

Can't we just avoid loading the JS it if the user is not authenticated?