Skip to content

Hotfix (to production): Catch Socket.io errors

Jano requested to merge hotfix_catch_socket_io_errors into master

Closes chat server needing to be auto-restarted right now because of the following Exception:

/var/www/beta/releases/656/chat/src/SocketController.ts:45
            throw new Error('not authorized');
                  ^
Error: not authorized
    at SocketController.readSessionId (/var/www/beta/releases/656/chat/src/SocketController.ts:45:19)
    at SocketController.onRegister (/var/www/beta/releases/656/chat/src/SocketController.ts:21:32)
    at Socket.<anonymous> (/var/www/beta/releases/656/chat/src/Framework/WebSocket/SocketIOServerFacade.ts:45:82)
    at Socket.emit (events.js:310:20)
    at Socket.EventEmitter.emit (domain.js:482:12)
    at /var/www/beta/releases/656/chat/node_modules/socket.io/lib/socket.js:528:12
    at processTicksAndRejections (internal/process/task_queues.js:79:11)
error Command failed with exit code 1.

What does this MR do?

In the old implementation, the 'not authorized' Error created when a client tried to register without a session id has not been thrown, but passed to socket.io to handle it. This MR catches all errors occuring in the SocketController and lets socket.io handle them. Errors thrown in the SocketController shouldn't stop the server from running.

How confident are you it won't break things if deployed?

Tested nothing, but I believe in it.

How to test

  1. Deploy
  2. See either no error stopping the server anymore or everything exploding in flames

Checklist

  • added a test, or explain why one is not needed/possible...
  • no unrelated changes
  • asked someone for a code review
  • joined #foodsharing-beta channel at https://slackin.yunity.org
  • added an entry to CHANGELOG.md (description, merge request link, username(s))
  • Once your MR has been merged, you are responsible to update the #foodsharing-beta Slack channel about what has been changed here. They will test your work in different browsers, roles or other settings
Edited by Chris Oelmueller

Merge request reports