Previous logic (LLM summary):
> The current logic always prefers captured over outcome, even when outcome is an Err. Consider: callback runs successfully → slot = Some(Ok(value)) → returns None → but then runInteraction's commit fails and the deferred errbacks. Now captured = Some(Ok(value)) and outcome = Err(...), and we'd return Ok(value) — silently swallowing the commit failure.
>
> Whether that's reachable depends on Synapse's retry behavior (it may retry the whole interaction on commit failure, re-running the callback). It's likely a narrow edge case, but the comment ("we only trust this slot once the deferred has fired") implies the slot is authoritative once fired, which isn't quite true if the deferred fires with an error after a successful callback run.
LLM summary:
func only ever needs to be moved to a thread and called there sequentially, never shared by & across threads:
- In the erasure, func is captured by-move into the Box<dyn Fn ... + Send> callback (only Send is required to box it that way).
- The Python pool moves that callback into the runInteraction DB-thread closure and calls it there; the Rust pool calls it within a single async task. Neither sends &func to multiple threads concurrently.
Prompt:
```
Compare this with a previous approach that was swapped out in `d9a111bdfaf51cd4cadfdc071f9956eec47aea43` (look at the removals). Anything we should change/simplify or something you like better/worse about that one? Give the new approach a review and simplify and make things more clear as necessary
```
Changes made (LLM summary):
1. #[async_trait] on the object-safe method. The core trait method is now async fn run_interaction_erased(...) -> ErasedResult instead of a hand-rolled fn ... -> BoxFuture<'a, _>. This is the convention already used by Transaction in the same file, and it removed the async move { ... }.boxed() wrapper (plus the BoxFuture/FutureExt imports) from both pool impls — their bodies are now plain async fns again.
2. Dropped the constrain / erase_interaction helper. I'd over-engineered the closure erasure out of a worry that higher-ranked-lifetime inference would fail. The previous code proved a direct let erased: ErasedInteraction = Box::new(move |txn| {...}) compiles — the type annotation on the binding is enough. ~30 lines gone.
3. Dropped the unneeded + Sync on the erased callback type. ErasedInteraction is + Send only now; the boxed callback is moved to a single DB thread and called there, never shared by reference.
```
[ERROR]
Traceback (most recent call last):
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/trial/runner.py", line 711, in loadByName
return self.suiteFactory([self.findByName(name, recurse=recurse)])
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/trial/runner.py", line 474, in findByName
obj = reflect.namedModule(searchName)
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/python/reflect.py", line 156, in namedModule
topLevel = __import__(name)
File "/home/eric/Documents/github/element/synapse/tests/__init__.py", line 24, in <module>
from synapse.util.patch_inline_callbacks import do_patch
File "/home/eric/Documents/github/element/synapse/synapse/__init__.py", line 31, in <module>
from synapse.util.rust import check_rust_lib_up_to_date
File "/home/eric/Documents/github/element/synapse/synapse/util/__init__.py", line 41, in <module>
from synapse.types import JsonDict
File "/home/eric/Documents/github/element/synapse/synapse/types/__init__.py", line 63, in <module>
from synapse.api.errors import Codes, SynapseError
File "/home/eric/Documents/github/element/synapse/synapse/api/errors.py", line 33, in <module>
from synapse.util.json import json_decoder
File "/home/eric/Documents/github/element/synapse/synapse/util/json.py", line 23, in <module>
from synapse.synapse_rust.events import JsonObject
File "/home/eric/Documents/github/element/synapse/synapse/logging/context.py", line 55, in <module>
from synapse.util.stringutils import random_string_insecure_fast
File "/home/eric/Documents/github/element/synapse/synapse/util/stringutils.py", line 31, in <module>
from synapse.api.errors import Codes, SynapseError
builtins.ImportError: cannot import name 'Codes' from partially initialized module 'synapse.api.errors' (most likely due to a circular import) (/home/eric/Documents/github/element/synapse/synapse/api/errors.py)
```
After looking into it, just a couple of things to pick a bone at in the
old wording,
which I thought could be clarified for when I next come to look at this
again.
- the claim that there's a fundamental difference; I'd argue there isn't
really, it's just by convention
on some mainstream distros. So I have changed this to 'typically'
- statements that some distros fetch dependencies at build time
(probably does happen, but
traditional distros make a point of not doing this for the reasons you'd
expect).
- This was probably meant to be talking about Debian, but my observation
based on sample size of 3 is that some crates are packaged natively,
others are vendored in the respective application's source package (like
they do for us) and sometime they patch the bounds a bit
There could probably be room to talk about how distros vendoring
packages is a maintenance burden on them,
but I guess it's a bit moot as we would struggle to conform to wide
enough bounds to make everyone
happy (and anyway; I expect the distros that vendor packages have the
tooling to make this easy to
update and we do keep on top of security updates and release
frequently...)
---
Spawning from discussion in
[`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$VttYPPUevn2S_W_rrzg2ZOXWI6aKebk2ganTgrLEWUc?via=jki.re&via=element.io&via=matrix.org)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Follows: #19487
Part of: MSC4354 whose experimental feature tracking issue is #19409
This PR implements the Sliding Sync (MSC4186) extension described in
MSC4354, allowing sliding sync clients
to receive sticky events in a reliable way.
The logic is much the same as for oldschool sync (implementation in
#19487),
although in the sliding sync extension, the client can choose their own
limit
and must control their own pagination through an extra token in the
extension request/response bodies.
Note this does not yet send down existing sticky events in the
room when the room has been newly-joined.
This newly-discovered gap is tracked at #19662 and will be addressed for
both current sync and MSC4186 SSS soon.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Co-authored-by: Eric Eastwood <erice@element.io>
Bringing it up to parity with the other 404 StoreErrors naming the table
name already.
More helpful response when debugging issues with incomplete relations.
Concretly: a user that got partly removed and then reinstated, missing
an entry in 'profiles' table.
This should also contribute to a better understanding of #2807 and #2173
Eric Eastwood