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

[chore] Refactor federatingDB.Undo, avoid 500 errors on Undo Like #3310

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

tsmethurst
Copy link
Contributor

@tsmethurst tsmethurst commented Sep 16, 2024

Description

If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.

If this is a documentation change, please briefly describe what you've changed and why.

This pull request does a bit of refactoring of undo.go in the federatingdb package, specifically to bring it in line with other more modern files in that package like reject.go.

Thing 1: Don't return 500 if we just can't get something because we never had it stored. This was happening frequently when Misskey and Friendica instances were sending us Undo's of Likes that we'd never processed, leading to annoying log messages like this:

timestamp="16/09/2024 07:58:45.971" func=server.init.func1.Logger.13.1 level=ERROR latency="39.358876ms" userAgent="Friendica 'Yellow Archangel' 2023.12-1542; https://social.gl-como.it" method=POST statusCode=500 path=/users/tobi/inbox clientIP=xxx.xxx.xxx.xxx pubKeyID=https://social.gl-como.it/profile/valhalla#main-key errors="Error #01: PostInboxScheme: error calling sideEffectActor.PostInbox: Undo: error undoing like: undoLike: error converting ActivityStreams Like to fave: getASObjectStatus: error getting object account from database: sql: no rows in result set\n" requestID=nhypkych040013sdj0rg msg="Internal Server Error: wrote 33B"

Now we just ignore such messages + return 202.

Thing 2: Have the federatingactor actually check for possible gtserror.WithCode instances, and use those instead of falling back to 500. I think we just forgot to hook this up, so we were returning lots of fancy different types of WithCode errors, but just squashing them all into 500 anyway :') Oops.

No real logic changes aside from that!

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have not leveraged AI to create the proposed changes.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc merged commit 0567b31 into main Sep 16, 2024
3 checks passed
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc deleted the federating_db_undo_refactor branch September 16, 2024 15:49
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