Add setting to send diagnostics back to SN

[ekzyis]

This should help to investigate #411.

This adds a new opt-in setting allow diagnostics:

I added a new endpoint /api/log where diagnostics data from devices of stackers which have this setting enabled are sent. This endpoint is unauthenticated for privacy reasons.

To know which events belong to the same client, a fancy random name is generated which is only known to the client. Hence if a stacker uses two different devices, it will show up with two different names. This should make sending these events pseudonymous since we could trace back to the individual stacker if the stacker tells us their fancy name. It would only be anonymous if this step is not reversible.

Additionally, anything sent to that endpoint is also sent into a Slack channel:

sample log in slack:

Since the endpoint is unauthenticated, you don't even need an account to send something to it. This may be a problem but if we require authentication, this seems antithetical to

without a doubt doesn't violate user privacy

-- #411 (comment)

Additionally, I think using authentication doesn't even really help against potential spam since anyone can easily create an account.

[ekzyis]

Asked for feedback here: https://stacker.news/items/248388

[huumn]

Here you check for navigator but you don't the line before. I think I recall navigator not being defined in some browsers. It might be best to bail out of os detection if navigator isn't defined.

[ekzyis]

Copied that code from SO. Not sure why there is this inconsistency.

Also, navigator should be supported by every browser. We may had this error because of SSR?

But I'll use window.navigator? anyway. Better be safe than sorry :)

[ekzyis]

Since we discussed hiding the setting and only enabling it for us manually ... should we just hide the setting?

The advantage would be that we don't need to explain what it does.

The disadvantage would be that it's less transparent that this exists and some stackers may actually want to help us.

I'll also try to fix the error boundary in this PR. So we'll get more useful information out of these changes already.

[huumn]

Since we discussed hiding the setting and only enabling it for us manually ... should we just hide the setting?

Nah let's keep it there. I'll do a pass on the copy before/after merging.

[ekzyis]

Using the Slack SDK now.

The only (minor, imo) issue is that messages from a client arrive out of order in slack since we don't wait until the previous slack requests are done before submitting new slack requests.

Doing this would require a mutex in the server or client afaict. We can't just use await somewhere since the slack requests are triggered in different API requests which are triggered at different points in the client code.

But I think this fine for the first iteration.

I also looked into the error boundary. I noticed that getDerivedStateFromError is triggered but not componentDidCatch. Also, ErrorBoundary has no access to me, logger etc. so it would require a more elaborate solution to send errors from there to /api/log.

So I think it's also not worth it to dig into that further - at least for now. We can use this as-is and see if that even works well. If too many people start using it, the messages being out of order may become more important so that should be fixed in that case first before anything else.

[huumn]

Overall it looks good. The only real concern I have with this is storing the log messages in the application db when this may be kind of noisy/a lot to store.

It might be worth removing the db storage until we have somewhere less permanent to store them. idk I need to think about this a little more.

[huumn]

Going to merge (and possibly comment out the db storage)