Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple instances of logger #346

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Fix multiple instances of logger #346

merged 1 commit into from
Aug 6, 2024

Conversation

heshammourad
Copy link
Owner

I have been noticing a lot of noise in the server logs recently with a lot of 400 Bad Request errors. I'm pretty sure those are from the snoowrap library (which I just found out has been archived), but I wasn't sure why there were so many of them. I dug a little deeper and found the errors being logged from files that didn't even interact with the library and realized that I each logger was logging unhandled exceptions and rejections, so there were a dozen entries for each error. I refactored the way logs are created, so that only the INDEX one tries logging unhandled exceptions/rejections.

Sorry for the noise in this CL, but I tried setting the line length in the Prettier config to shorter than the ESLint config, and it did a much better job of placing line breaks, so we only need 3 of the max-len statements in the app. By running Prettier -> ESLint, it did a lot of reformatting to the files, but the only files I changed the logic in are logger.js and index.js (to pass the new params). You should be able to safely ignore the remaining files.

Copy link
Collaborator

@rissois rissois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't run it, but the code looks good and the formatting a lot cleaner, thanks.

I don’t think we need to worry about DOS attacks, but I do want to flag this warning: https://github.com/heshammourad/vexillology-contests/security/code-scanning/21

Also, what does the archiving of snoowrap mean for us?

@heshammourad
Copy link
Owner Author

I have captured #11 for the missing rate limited from way back in the beginning, so I imagine it's not a pressing concern, but something we should get to eventually.

As for the snoowrap being archived, it doesn't affect us that much. It looks like it happened because the maintainers didn't have enough time to support it, and there were no volunteers to take on the work. There was also the expectation that a better library would eventually be provided by Reddit as part of that developers network thing, but who knows when that might happen. We only really need it to get the username when a user logs in, so we could remove it and replace with our own implementation for just this part. It's also used for the older contests to scrape the data, but I imagine that is very rarely happening these days.

@heshammourad heshammourad merged commit 648c4bb into master Aug 6, 2024
1 of 2 checks passed
@heshammourad heshammourad deleted the bug/logger branch August 6, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants