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

Decide if the support for additional fields in the spam checker API is here to stay or should be removed #17668

Open
nico-famedly opened this issue Sep 5, 2024 · 0 comments

Comments

@nico-famedly
Copy link
Contributor

nico-famedly commented Sep 5, 2024

Context: 11f8114

The above commit added support for returning additional fields from the spam checker API. This was marked as experimental and therefore not documented. It has now been 2 years and we should probably decide, if this will stay in the API or if the support for returning additional fields will be removed.

From my understanding, there are a few use cases, where the support for additional fields is useful:

  • It allows you to attach domain specific errors to various actions. For example usually an invite just fails with "Forbidden" without any further details. In some cases enterprises or especially eager users might implement their own spam checker to implement domain specific rules for invites. In those cases it can be useful to attach extra information to rejected invites, that clients from those domains can understand, either for debugging or for showing more specific error messages to users.
  • It allows you to overwrite the error and errcode fields of the SynapseError created from the spam checker rejection. This can be used as a spec compatible way to provide better error messages. For example one could change the message to "Invite to user is rejected because they are on a known spam server" or "You can't create another DM with this user, please leave the old DM first".

Both of these options bear their own risks. Additional, non-spec fields, may cause users of the spam checker API to diverge from the spec and currently no validation is done, that those fields follow Matrix namespacing rules. On the other hand overwriting the error message is more standardized, but it also could be too powerful and if that is the only use case, a simpler API might be preferable compared to accepting an arbitrary tuple.

So the question is:

  • Should the support for additional fields get removed?
  • Should it stay but its functionality restricted to only allow one of the 2 uses above?
  • Are there any users, that depend on this API already?
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

No branches or pull requests

1 participant