48 need to log errors to server log in messagingts

[zuechai]

PR for issue #48. It appears to be logging correctly. Right now, the stack is not printed to the log, but that can be easily added if desired.

[railway-app[bot]]

This PR is being deployed to Railway 🚅

admin-portal: ◻️ REMOVED

[mattsahn]

I triggered the error messages by hitting buttons that I knew would generate error in the PR link. I see the errors show up in the log, which is good, but each shows up twice, which I wasn't expecting. Why is that? Does it get retried automatically? Or something else going on?

[zuechai]

@mattsahn Looking into why the errors are printed twice and I think it is happening in the mutationFn() of Messaging.tsx in the Client. I'm going to read through React Query docs to see if that theory makes any sense. I'll also go through the logger configs again to see if there's something I need to change there.

[mattsahn]

@mattsahn Looking into why the errors are printed twice and I think it is happening in the mutationFn() of Messaging.tsx in the Client. I'm going to read through React Query docs to see if that theory makes any sense. I'll also go through the logger configs again to see if there's something I need to change there.

Not a big deal, mostly curious to understand what's happening there. I'm not a frontend dev at all, btw :) @jasoncavanaugh, feel free to jump in and enlighten us and review some code if you're free!

[jasoncavanaugh]

I would have to look more closely at the logs, but react query defaults to retrying fetches when they fail, so that could be why you are seeing multiple error logs consecutively for the same endpoint

[zuechai]

Ok, so this has been a mess of a PR. I'm sorry for all my silly mistakes and my confusion.

An alternative for logging errors generally is adding the logger to the error middleware. Then anytime an error is thrown it will be logged by the middleware. Only downside I see is Express doesn't send responses with status 404 to the error handler middleware. This would look something like this:

Otherwise, here are the changes I'm planning to make. I just want to make sure this list is complete and accurate:

• add back throws removed as they were before this PR
• change all logger.error(new Error("message")) to `logger.error("message")
• change to ${resp.status} ${MAKE_ERROR_MESSAGE} from resp.status + " " + MAKE_ERROR_MESSAGE

Let me know your thoughts. Thanks @mattsahn @jasoncavanaugh

[jasoncavanaugh]

Ok, so this has been a mess of a PR. I'm sorry for all my silly mistakes and my confusion.

An alternative for logging errors generally is adding the logger to the error middleware. Then anytime an error is thrown it will be logged by the middleware. Only downside I see is Express doesn't send responses with status 404 to the error handler middleware. This would look something like this:

Otherwise, here are the changes I'm planning to make. I just want to make sure this list is complete and accurate:

• add back throws removed as they were before this PR
• change all logger.error(new Error("message")) to `logger.error("message")
• change to ${resp.status} ${MAKE_ERROR_MESSAGE} from resp.status + " " + MAKE_ERROR_MESSAGE

Let me know your thoughts. Thanks @mattsahn @jasoncavanaugh

Yep this looks good to me! I opened up a separate issue for making the way we throw errors more consistent, so that combined with your changes will probably help a lot

[zuechai]

I'm going to close this PR and resubmit a clean branch as the only change that actually needs to be made is adding the logger to the middleware and nothing to messaging.ts