Rough implementation of Sentry for error logging

[ayim]

Changes Made

• Added sentry.io to project dependencies for error logging
• Imported sentry to screens for error logging
• Added error logging to catch statements on HomeAuth.js

Testing Done

• Tested: Cleared async storage, launch app, login, access chat screen, send a message
• Not Tested: Chat failure paths
  • Couldn't join channel
  • Couldn't invite user
  • Error in creating channel
  • Error getting token on refresh
  • Error while trying to create Twilio client

Pending Work

• Add internal user id to error reporting https://docs.sentry.io/enriching-error-data/context/?platform=browser#capturing-the-user
  Attempted but unsuccessful
• Add more sophisticated error handling to async authentication calls on HomeAuth.js

[ayim]

Once Sentry account for HeyMentor has been created, need to perform these steps:
Sentry.enableInExpoDevelopment = true;
Sentry.config('your Public DSN goes here').install();

https://docs.expo.io/versions/latest/guides/using-sentry/

[callumdmay]

Not that familiar with sentry, but is there a catch all pattern for failures? I know for example this sentry library catches all uncaught javascript exceptions (in a browser environment). Wondering if we want something similar, so in the future if someone forgets to put one of those log statements in the catch block.

[bongiovimatthew-microsoft]

Yeah I agree it would be ideal if there were a catchall pattern we could use. Aaron and I spoke about that briefly, and it sounded like there wasn't one for ReactNative, but @ayim can correct me if I'm wrong.

[ayim]

@callumdmay from experimenting with Sentry, seems to me that the catchall for JS exceptions is enabled by default.

Looks like there were a number of JS exceptions not in a try/catch that were logged - the first two in the list were simple syntax errors.

[bongiovimatthew]

Okay awesome. Do you guys think we should remove the generic logging statements then, since they will be caught anyway?

I'm also thinking how we want to correlate uncaught error logs to the specific user. We have the user ID we can add in the logs, but I don't know if we can do that for the catchall logs...

[ayim]

RE: generic logging statements, I think they may be useful to see in console while in development.
There's also an easy flag that we can use to disable Sentry during development.

RE: correlating uncaught errors to a user, I was having issues getting that working for try > catch logs. Can take a look at whether that's something we can add to the catchall logs

[callumdmay]

When would it be necessary to correlate an error to a specific user? I'm worried this could be a privacy issue. Shouldn't the logging be able to tell us all we need to know to fix the regression?

With regards to keeping the console logging during development, I actually see this as an anti pattern, logging everything to console is actually really annoying when you want to debug something by logging to console because there's already so much noise. On the api for example I have no-console enabled on linting, but I still use console logs plenty to debug, but just don't commit them.

[bongiovimatthew]

I agree on the point of having no console logs.

For correlating logs to a user, I think that's going to be important, especially when we have several hundred users who may all be generating different errors at different times. The scenario we will want to handle is when a user emails us saying they are seeing some issue, we will need to know which failure logs to look at.

I don't think there should be a privacy issue on this point, because the error logs shouldn't contain information that we don't already have.

[callumdmay]

@ayim @bongiovimatthew whats the status of this?

[bongiovimatthew]

I'll go ahead and try to integrate this soon and put the pending items onto the trello stack

[ayim]

Hey, sorry for dropping the ball here.
@bongiovimatthew is it cool with you if I remove the console logging tonight?
Can look at getting User IDs working with Sentry as well.

[bongiovimatthew]

No worries @ayim. If you don't mind, could you switch to working off lastMessageLogic? I removed all the console logging for that PR, and have some spots marked for Sentry.

[ayim]

Will do. Just wrapping up my internship this week so availability be great next week.