[bug] Sending a Follow Accept without an Actor results in 500

[algernon]

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

I am implementing some ActivityPub things for Forgejo, and part of that work requires a Forgejo account to be followable by other Fediverse software. I'm testing primarily with GoToSocial, because the debug messages are usually enough to figure out what I'm doing wrong.

In this case, I was trying to make the follow workflow function, but accidentally left out the Actor property of the Accept activity I send back to GoToSocial. GoToSocial responds with a HTTP 500 in this case, even though the fault is clearly on the client side: this should be a HTTP 400.

What's your GoToSocial Version?

0.16.0

GoToSocial Arch

amd64, binary via NixOS

What happened?

I sent the following to my GoToSocial instance:

{
  "@context": "https://www.w3.org/ns/activitystreams",
  "id": "https://shoes.forgejo.madhouse-project.org/api/v1/activitypub/user-id/1/follows/a232ef3b-2d2f-4e0e-a8f6-fde9745575ba",
  "type": "Accept",
  "object": {
    "id": "https://gts.shoes.forgejo.madhouse-project.org/users/algernon/follow/01J8Q8HZV3FFKSP36HXGYVS7NZ",
    "type": "Follow",
    "to": [
      "https://shoes.forgejo.madhouse-project.org/api/v1/activitypub/user-id/1"
    ],
    "actor": "https://gts.shoes.forgejo.madhouse-project.org/users/algernon",
    "object": "https://shoes.forgejo.madhouse-project.org/api/v1/activitypub/user-id/1"
  }
}

Notice that while the object does have an actor, the wrapping Accept does not. GtS rewarded me with an 500.

What you expected to happen?

Since this is a client error, I would have expected a 400 bad request instead.

How to reproduce it?

Take your favourite Fediverse server, make it send an Accept without an actor, then try to follow an account there.

Anything else we need to know?

No response

[tsmethurst]

Oh thanks for opening. There's a few cases where we return 500 when it should be 4xx, since 500 is sort of our "default" error when nothing else has been put in place. So a 500 from GtS isn't necessarily an "oh fuck" kind of issue, it just means we didn't handle something especially neatly. In this instance it definitely should be changed though.

[algernon]

Yeah, figured that's the case, this is just a cosmetic little thing, which likely very few people will see, those who manage to construct a proper AP activity, but manage to leave out the Actor. =)