superseriousbusiness / gotosocial

Fast, fun, small ActivityPub server.
https://docs.gotosocial.org
GNU Affero General Public License v3.0
3.7k stars 316 forks source link

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

Closed tsmethurst closed 1 week ago

tsmethurst commented 1 week ago

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).