Fix import in module_api module and docs on the new check_event_for_spam signature

docs/modules/spam_checker_callbacks.md

@@ -18,12 +18,12 @@ _Changed in Synapse v1.60.0: `synapse.module_api.NOT_SPAM` and `synapse.module_a
 async def check_event_for_spam(event: "synapse.module_api.EventBase") -> Union["synapse.module_api.NOT_SPAM", "synapse.module_api.errors.Codes", str, bool]
 ```
 
-Called when receiving an event from a client or via federation. The callback must return either:
+Called when receiving an event from a client or via federation. The callback must return one of:
   - `synapse.module_api.NOT_SPAM`, to allow the operation. Other callbacks may still 
     decide to reject it.
   - `synapse.module_api.errors.Codes` to reject the operation with an error code. In case
     of doubt, `synapse.module_api.errors.Codes.FORBIDDEN` is a good error code.
-  - (deprecated) a `str` to reject the operation and specify an error message. Note that clients
+  - (deprecated) a non-`Codes` `str` to reject the operation and specify an error message. Note that clients
     typically will not localize the error message to the user's preferred locale.
   - (deprecated) `False`, which is the same as returning `synapse.module_api.NOT_SPAM`.
   - (deprecated) `True`, which is the same as returning `synapse.module_api.errors.Codes.FORBIDDEN`.

synapse/events/spamcheck.py

@@ -47,11 +47,8 @@
     Awaitable[
         Union[
             str,
-            Codes,
             # Deprecated
             bool,
-            # Deprecated
-            str,
         ]
     ],
 ]
@@ -272,7 +269,7 @@ def register_callbacks(
 
     async def check_event_for_spam(
         self, event: "synapse.events.EventBase"
-    ) -> Union[Codes, str]:
+    ) -> Union[str]:
         """Checks if a given event is considered "spammy" by this server.
 
         If the server considers an event spammy, then it will be rejected if
@@ -295,7 +292,7 @@ async def check_event_for_spam(
             with Measure(
                 self.clock, "{}.{}".format(callback.__module__, callback.__qualname__)
             ):
-                res: Union[Codes, str, bool] = await delay_cancellation(callback(event))
+                res = await delay_cancellation(callback(event))
                 if res is False or res == self.NOT_SPAM:
                     # This spam-checker accepts the event.
                     # Other spam-checkers may reject it, though.
@@ -307,6 +304,16 @@ async def check_event_for_spam(
                 else:
                     # This spam-checker rejects the event either with a `str`
                     # or with a `Codes`. In either case, we stop here.
+                    if not isinstance(res, str):
+                        # Make sure we give the SynapseError a string as the error
+                        # message.
+                        # mypy complains that we can't reach this code because of the
+                        # return type in CHECK_EVENT_FOR_SPAM_CALLBACK, but we don't know
+                        # for sure that the module actually returns it.
+                        res = (  # type: ignore[unreachable]
+                            "This message has been rejected as probable spam"
+                        )
+
                     return res
 
         # No spam-checker has rejected the event, let it pass.

synapse/handlers/message.py

@@ -886,16 +886,19 @@ async def create_and_send_nonmember_event(
             )
 
             spam_check_result = await self.spam_checker.check_event_for_spam(event)
-            if isinstance(spam_check_result, Codes):
-                raise SynapseError(
-                    403,
-                    "This message had been rejected as probable spam",
-                    spam_check_result,
-                )
-            elif (
-                isinstance(spam_check_result, str)
-                and spam_check_result != self.spam_checker.NOT_SPAM
-            ):
+            if spam_check_result != self.spam_checker.NOT_SPAM:
+                # We need to check whether the return value is an error code
+                # specifically for backwards compatibility, since values in Codes
+                # are also strings and this callback allows modules to return
+                # arbitrary strings to be used in SynapseErrors (which is now
+                # deprecated).
+                if isinstance(spam_check_result, Codes):
+                    raise SynapseError(
+                        403,
+                        "This message has been rejected as probable spam",
+                        spam_check_result,
+                    )
+
                 raise SynapseError(
                     403,
                     spam_check_result,