superseriousbusiness / gotosocial

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

[bug] Domain block edits can result in an undefined cache state #1157

Closed trysdyn closed 1 year ago

trysdyn commented 1 year ago

Describe the bug with a clear and concise description of what the bug is.

Attempting to edit a domain block to add a new private comment put GtS in a state where I could no longer work with domain blocks until a restart. Attempting to edit blocks silently did not reflect in the DB and attempting to delete blocks worked, but produced errors in the log. Restarting fixed all issues.

What's your GoToSocial Version?

v0.5.0 git-8942a70 (note the incorrect point release number; this is a <1wk old pull)

GoToSocial Arch

amd64 binary

Browser version

Any

What happened?

GtS cache/db interaction began acting strangely when working with domain blocks. Replication steps below explain better.

After trying to edit a domain block to add a comment, changes stopped being reflected until I tried to delete the block, at which point I received a web frontend JSON error and this error emitted in the logs (I have scrubbed the domain I actually blocked and replaced it with BLOCKED.DOMAIN):

timestamp="27/11/2022 13:13:05.367" func=router.loggingMiddleware.func1 level=ERROR stacktrace="result.structKeys.get()\n\tcodeberg.org/gruf/go-cache/v3@v3.1.8/result/key.go:25\nresult.(*Cache[...]).Invalidate()\n\tcodeberg.org/gruf/go-cache/v3@v3.1.8/result/cache.go:250\nbundb.(*domainDB).DeleteDomainBlock()\n\tgithub.com/superseriousbusiness/gotosocial/internal/db/bundb/domain.go:128\nadmin.(*processor).DomainBlockDelete()\n\tgithub.com/superseriousbusiness/gotosocial/internal/processing/admin/deletedomainblock.go:51\nprocessing.(*processor).AdminDomainBlockDelete()\n\tgithub.com/superseriousbusiness/gotosocial/internal/processing/admin.go:70\nadmin.(*Module).DomainBlockDELETEHandler()\n\tgithub.com/superseriousbusiness/gotosocial/internal/api/client/admin/domainblockdelete.go:97\ngin.(*Context).Next()\n\tgithub.com/gin-gonic/gin@v1.8.1/context.go:173\nsecurity.(*Module).TokenCheck()\n\tgithub.com/superseriousbusiness/gotosocial/internal/api/security/tokencheck.go:120\ngin.(*Context).Next()\n\tgithub.com/gin-gonic/gin@v1.8.1/context.go:173\n" msg="recovered panic: unknown lookup: \"BLOCKED.DOMAIN\""

Despite the error, deleting the block worked but trying to re-add it was silently not reflected in the DB. I could fix this state by restarting GtS, leading me to believe it's a caching issue.

What you expected to happen?

Given the repro steps below...

How to reproduce it?

  1. New instance
  2. Enter $DOMAIN/admin/federation
  3. Add a new domain block with no comments
  4. Click the domain block, add a new Private Comment, save
  5. Comment will not be saved (verified with API and DB query)
  6. Delete the domain block
  7. Receive web frontend error "JSON Unexpected end of data"
  8. Receive logged error as noted above
  9. Block will delete regardless of error
  10. Re-add block with private comment
  11. Receive no error but the block will not be added (verified with API and DB query)
  12. Restart GtS
  13. Add domain block with private comment
  14. Domain block will be added correctly

Anything else we need to know?

I know I shouldn't be using main/HEAD in production. But hey, I get to catch stuff like this :)

tsmethurst commented 1 year ago

Thanks for opening :)

NyaaaWhatsUpDoc commented 1 year ago

@trysdyn thanks for reporting! spotted it right away, was a very simple little bug i introduced when moving to our updated caching system. it would successfully delete the domain block from the database, but then the call to invalidate it from the cache was missing a required "lookup" key (it's a fancy system that lets us cache objects under multiple different keys)

fix should be in main now :). let us know how you get on!

tsmethurst commented 1 year ago

Closed by https://github.com/superseriousbusiness/gotosocial/pull/1158

[edit] or at least, it should be -- feel free to reopen the issue if it's not fixed ofc

trysdyn commented 1 year ago

Confirmed closed yes; thanks so much :)