MSC4140: update error responses

[AndrewFerr]

• Impose limit of scheduled delayed events
• Update error codes to match latest draft of MSC4140

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct (run the linters)

[MadLittleMods]

Explain more why and what do do.

Enforce max delayed event config to be positive

If delayed events are to be disabled, it should be done by leaving the
max_event_delay_duration config unspecified, not by setting the delayed
event limit to 0.

-- 43e14e9

[AndrewFerr]

267a7e5 rewords the error message to be a bit more descriptive, and to follow the same format used by similar errors in this module.

[MadLittleMods]

Per the original intentions, I was more looking for something like this:

Suggested change

	raise ConfigError(
	"Expected a positive value", ("max_event_delay_duration",)
	)
	raise ConfigError(
	"Expected a non-zero, positive value for the delay duration. To disable delayed events, leave `max_event_delay_duration` unspecified.", ("max_event_delay_duration",)
	)

It seems like ideally, we would have had this kind of structure for delayed event config:

delayed_events:
  enabled: true
  max_event_delay_duration: 24h
  max_delayed_events_per_user: 100

---

If we're going with 0 as a valid value to disable delayed events, I guess the updated error message works ⏩

[AndrewFerr]

That new config does look better, though I'd prefer using a dedicated PR to change it, given that max_event_delay_duration has been around for a while now & moving it would be a breaking change.

I'd also prefer to keep config suggestions in the documentation instead of error messages, to reduce churn on code changes while the MSC is still unstable.

[MadLittleMods]

It looks like msc4140_enabled was added. Are we interested in getting the config shape correct from the beginning?

Generally, we try not to break peoples homeserver config. And changing this later means even more complication for the backwards compatibility.

[AndrewFerr]

msc4140_enabled isn't a real config key. It's just a computed value to make it easier to have /versions report whether delayed events are enabled via the other config keys, done since 0859de0. (It is also used in c33d443.)

Which is to say, the config shape has not changed, nor do I intend to change it.

[MadLittleMods]

(review pass)

[AndrewFerr]

To view just the merge conflict resolution made by 0859de0, you can use this:

diff <(git diff $(git merge-base develop fair/msc4140-error-updates^1) fair/msc4140-error-updates^1) <(git diff develop fair/msc4140-error-updates)