Details

Reviewers

tomek
ashoat

Commits

Summary

This differential is created in response to the following linear issue: https://linear.app/comm/issue/ENG-4451/improve-logging-for-olm-session-creation-failure. Olm session creation failure is a seriious concern and current error handling does
not provide meaningful information for debugging purposes.

Test Plan

Delete olm sessions table from the database. Try to log in from mobile app. Ensure that meaningful logs are printed to the console.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 25 2023, 4:33 AM

marcin requested review of this revision.Jul 25 2023, 5:09 AM

tomek added inline comments.

keyserver/src/creators/olm-session-creator.js
27 ↗	(On Diff #29010)	We probably shouldn't log cookieID

This revision is now accepted and ready to land.Jul 25 2023, 6:17 AM

keyserver/src/creators/olm-session-creator.js
27 ↗	(On Diff #29010)	Knowing the `cookieID` for which the failure occurred might be helpful for debugging purposes. Does it create security issues?

keyserver/src/creators/olm-session-creator.js
27 ↗	(On Diff #29010)	Can you please keep all lines to a max of 80 chars? I think it's okay to log the cookie ID on the keyserver. Presumably the keyserver owner has access to this information anyways via MariaDB I wonder if relying on Node to render the exception might be better than `getMessageForException`, eg. `console.warn('message', e)`