mirror of
https://github.com/element-hq/synapse.git
synced 2026-11-11 14:27:48 +00:00
Fix review findings in the Rust transaction sender
- Only build the Rust sender from a real `RustFederationClient`. A test that mocks the federation client no longer fails with a `TypeError`. - Log the `TX` and response lines under Python's logger names, so log configs that name those loggers still apply. - Move the PDUs and EDUs into the request body, so a transaction holds one copy of them while it is retried, besides the canonical JSON. - Drop the unused `time_now` from `Event::to_pdu_value`. - Set the compare mode per `TransactionManager` in the tests that use it, not on the class. - Test that the tracing headers reach the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5b7Ruy4cgt27Mjq6Ptxbb
This commit is contained in:
1 parent
5aff3307b2
commit
b8d8d5e015
8 files changed
+186
-91
No files matched your search
+9
-22
@@ -747,22 +747,13 @@ impl Event {
|
||||
&self.internal_metadata
|
||||
}
|
||||
|
||||
/// The event as `get_pdu_json` returns it, built without the GIL.
|
||||
/// The event as `get_pdu_json()` returns it, built without the GIL.
|
||||
///
|
||||
/// With `time_now`, an `age_ts` in `unsigned` becomes `age`. An `age_ts`
|
||||
/// out of `i64` range is left as it is (Python would raise). The Rust
|
||||
/// `unsigned` never holds `redacted_because`, so there is nothing to strip.
|
||||
pub fn to_pdu_value(&self, time_now: Option<i64>) -> serde_json::Result<Value> {
|
||||
let mut value = serde_json::to_value(&self.parsed_event)?;
|
||||
if let Some(time_now) = time_now {
|
||||
if let Some(unsigned) = value.get_mut(UNSIGNED).and_then(Value::as_object_mut) {
|
||||
if let Some(age_ts) = unsigned.get(AGE_TS).and_then(Value::as_i64) {
|
||||
unsigned.insert(AGE.to_owned(), Value::from(time_now - age_ts));
|
||||
unsigned.remove(AGE_TS);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(value)
|
||||
/// Without a `time_now`, Python leaves `age_ts` in `unsigned`, and so does
|
||||
/// this. The Rust `unsigned` never holds `redacted_because`, so there is
|
||||
/// nothing to strip.
|
||||
pub fn to_pdu_value(&self) -> serde_json::Result<Value> {
|
||||
serde_json::to_value(&self.parsed_event)
|
||||
}
|
||||
|
||||
/// Returns the `redacts` field of this event, if it has one.
|
||||
@@ -1124,7 +1115,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn to_pdu_value_rewrites_age_ts() {
|
||||
fn to_pdu_value_is_the_stored_json() {
|
||||
let json = r#"{"auth_events":[],"prev_events":[],"type":"m.room.message","sender":"@a:example.com","content":{"body":"hi"},"depth":3,"room_id":"!r:example.com","origin_server_ts":5,"hashes":{"sha256":"aaa"},"signatures":{},"unsigned":{"age_ts":1000}}"#;
|
||||
let event = Event::from_json_str(
|
||||
json,
|
||||
@@ -1134,12 +1125,8 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Without a time, the PDU is the event JSON as stored.
|
||||
// `age_ts` stays in `unsigned`, as `get_pdu_json()` leaves it.
|
||||
let as_stored: Value = serde_json::from_str(json).unwrap();
|
||||
assert_eq!(event.to_pdu_value(None).unwrap(), as_stored);
|
||||
|
||||
let pdu = event.to_pdu_value(Some(1500)).unwrap();
|
||||
assert_eq!(pdu["unsigned"], serde_json::json!({"age": 500}));
|
||||
assert_eq!(pdu["content"], as_stored["content"]);
|
||||
assert_eq!(event.to_pdu_value().unwrap(), as_stored);
|
||||
}
|
||||
}
|
||||
@@ -52,6 +52,12 @@ const CANONICALJSON_MAX_INT: i64 = (1 << 53) - 1;
|
||||
/// `CANONICALJSON_MIN_INT`.
|
||||
const CANONICALJSON_MIN_INT: i64 = -CANONICALJSON_MAX_INT;
|
||||
|
||||
/// The logger of Python's `TX [dest] {txn}` lines. Rust logs them under the
|
||||
/// same name, so log configs that set its level still apply.
|
||||
const TX_LOG: &str = "synapse.federation.sender.transaction_manager";
|
||||
/// The logger of Python's lines about reading the response.
|
||||
const CLIENT_LOG: &str = "synapse.http.matrixfederationclient";
|
||||
|
||||
/// The EDU types whose contents the issue 8631 debug log records.
|
||||
const DEVICE_UPDATE_EDUS: [&str; 2] = ["m.device_list_update", "m.signing_key_update"];
|
||||
|
||||
@@ -72,18 +78,23 @@ impl Transaction {
|
||||
/// The request body: `Transaction.get_dict`. PDUs with a depth canonical
|
||||
/// JSON cannot hold are dropped, and `edus` is left out when empty.
|
||||
pub fn body(&self) -> Value {
|
||||
self.clone().into_body()
|
||||
}
|
||||
|
||||
/// As [`Transaction::body`], moving the PDUs and EDUs into it.
|
||||
pub fn into_body(self) -> Value {
|
||||
let mut pdus = self.pdus;
|
||||
pdus.retain(has_valid_depth);
|
||||
|
||||
let mut body = Map::new();
|
||||
body.insert("origin".to_owned(), Value::from(self.origin.as_str()));
|
||||
body.insert("origin".to_owned(), Value::from(self.origin));
|
||||
body.insert(
|
||||
"origin_server_ts".to_owned(),
|
||||
Value::from(self.origin_server_ts),
|
||||
);
|
||||
body.insert(
|
||||
"pdus".to_owned(),
|
||||
Value::Array(filter_pdus_for_valid_depth(&self.pdus)),
|
||||
);
|
||||
body.insert("pdus".to_owned(), Value::Array(pdus));
|
||||
if !self.edus.is_empty() {
|
||||
body.insert("edus".to_owned(), Value::Array(self.edus.clone()));
|
||||
body.insert("edus".to_owned(), Value::Array(self.edus));
|
||||
}
|
||||
Value::Object(body)
|
||||
}
|
||||
@@ -96,32 +107,40 @@ impl Transaction {
|
||||
|
||||
/// The prefix of the transaction's log lines, `TX [dest] {txn}`.
|
||||
fn log_prefix(&self) -> String {
|
||||
format!("TX [{}] {{{}}}", self.destination, self.transaction_id)
|
||||
log_prefix(&self.destination, &self.transaction_id)
|
||||
}
|
||||
}
|
||||
|
||||
/// `TX [dest] {txn}`.
|
||||
fn log_prefix(destination: &str, transaction_id: &str) -> String {
|
||||
format!("TX [{destination}] {{{transaction_id}}}")
|
||||
}
|
||||
|
||||
/// `filter_pdus_for_valid_depth`: keep the PDUs whose `depth` is an integer
|
||||
/// that canonical JSON allows.
|
||||
pub fn filter_pdus_for_valid_depth(pdus: &[Value]) -> Vec<Value> {
|
||||
pdus.iter()
|
||||
.filter(|pdu| {
|
||||
let Some(depth) = pdu.get("depth") else {
|
||||
return false;
|
||||
};
|
||||
// Python compares whatever is there. An event's depth is always
|
||||
// an integer, but compare a float as Python would.
|
||||
if let Some(depth) = depth.as_i64() {
|
||||
(CANONICALJSON_MIN_INT..=CANONICALJSON_MAX_INT).contains(&depth)
|
||||
} else if let Some(depth) = depth.as_f64() {
|
||||
(CANONICALJSON_MIN_INT as f64..=CANONICALJSON_MAX_INT as f64).contains(&depth)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
})
|
||||
.filter(|pdu| has_valid_depth(pdu))
|
||||
.cloned()
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Whether the PDU has a `depth` that canonical JSON allows.
|
||||
fn has_valid_depth(pdu: &Value) -> bool {
|
||||
let Some(depth) = pdu.get("depth") else {
|
||||
return false;
|
||||
};
|
||||
// Python compares whatever is there. An event's depth is always an
|
||||
// integer, but compare a float as Python would.
|
||||
if let Some(depth) = depth.as_i64() {
|
||||
(CANONICALJSON_MIN_INT..=CANONICALJSON_MAX_INT).contains(&depth)
|
||||
} else if let Some(depth) = depth.as_f64() {
|
||||
(CANONICALJSON_MIN_INT as f64..=CANONICALJSON_MAX_INT as f64).contains(&depth)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// The remote's answer to a transaction.
|
||||
#[derive(Debug)]
|
||||
pub enum TransactionResponse {
|
||||
@@ -141,7 +160,7 @@ pub fn acknowledged_pdus(log_prefix: &str, response: &Map<String, Value>) -> Vec
|
||||
return Vec::new();
|
||||
};
|
||||
let Some(pdus) = pdus.as_object() else {
|
||||
log::warn!("{log_prefix} Remote returned invalid type for `pdus`");
|
||||
log::warn!(target: TX_LOG, "{log_prefix} Remote returned invalid type for `pdus`");
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
@@ -151,7 +170,10 @@ pub fn acknowledged_pdus(log_prefix: &str, response: &Map<String, Value>) -> Vec
|
||||
Some(result) if !result.contains_key("error") => {
|
||||
acknowledged.push(event_id.clone());
|
||||
}
|
||||
_ => log::warn!("{log_prefix} Remote returned error for {event_id}: {result}"),
|
||||
_ => log::warn!(
|
||||
target: TX_LOG,
|
||||
"{log_prefix} Remote returned error for {event_id}: {result}"
|
||||
),
|
||||
}
|
||||
}
|
||||
acknowledged
|
||||
@@ -182,6 +204,7 @@ impl TransactionSender {
|
||||
pub fn build(&self, destination: &str, pdus: Vec<Value>, edus: Vec<Value>) -> Transaction {
|
||||
let transaction_id = self.next_txn_id.fetch_add(1, Ordering::Relaxed).to_string();
|
||||
log::debug!(
|
||||
target: TX_LOG,
|
||||
"TX [{destination}] {{{transaction_id}}} Attempting new transaction (pdus: {}, edus: {})",
|
||||
pdus.len(),
|
||||
edus.len()
|
||||
@@ -198,29 +221,34 @@ impl TransactionSender {
|
||||
|
||||
/// Send `transaction` and read the remote's answer.
|
||||
///
|
||||
/// The PDUs and EDUs move into the request body, so the transaction holds
|
||||
/// one copy of them while it is retried, besides the canonical JSON.
|
||||
/// `extra_headers` carries the tracing span. The request's attempts and
|
||||
/// response codes go to `stats`, for the Python counters.
|
||||
pub async fn send(
|
||||
&self,
|
||||
transaction: &Transaction,
|
||||
transaction: Transaction,
|
||||
extra_headers: Vec<(String, String)>,
|
||||
stats: &mut RequestStats,
|
||||
) -> Result<TransactionResponse, ClientError> {
|
||||
let log_prefix = transaction.log_prefix();
|
||||
log::info!(
|
||||
target: TX_LOG,
|
||||
"{log_prefix} Sending transaction [{}], (PDUs: {}, EDUs: {})",
|
||||
transaction.transaction_id,
|
||||
transaction.pdus.len(),
|
||||
transaction.edus.len(),
|
||||
);
|
||||
log_device_list_updates(transaction);
|
||||
log_device_list_updates(&transaction);
|
||||
|
||||
let transaction_id = transaction.transaction_id.clone();
|
||||
let destination = transaction.destination.clone();
|
||||
let request = FederationRequest {
|
||||
method: Method::PUT,
|
||||
destination: transaction.destination.clone(),
|
||||
destination: destination.clone(),
|
||||
path: transaction.path(),
|
||||
query: String::new(),
|
||||
body: Some(transaction.body()),
|
||||
body: Some(transaction.into_body()),
|
||||
rewrite_age_ts: false,
|
||||
extra_headers,
|
||||
};
|
||||
@@ -238,18 +266,14 @@ impl TransactionSender {
|
||||
Ok(response) => response,
|
||||
Err(err) => {
|
||||
if let ClientError::Http(http) = &err {
|
||||
log::info!("{log_prefix} got {} response", http.code);
|
||||
log::info!(target: TX_LOG, "{log_prefix} got {} response", http.code);
|
||||
}
|
||||
return Err(err);
|
||||
}
|
||||
};
|
||||
|
||||
let body = self
|
||||
.read_json(
|
||||
response,
|
||||
&transaction.destination,
|
||||
self.client.config().default_timeout,
|
||||
)
|
||||
.read_json(response, &destination, self.client.config().default_timeout)
|
||||
.await?;
|
||||
|
||||
let value = match serde_json::from_slice::<Value>(&body) {
|
||||
@@ -258,9 +282,8 @@ impl TransactionSender {
|
||||
};
|
||||
let Value::Object(value) = value else {
|
||||
log::warn!(
|
||||
"{{{}}} [{}] Failed to parse response",
|
||||
transaction.transaction_id,
|
||||
transaction.destination
|
||||
target: CLIENT_LOG,
|
||||
"{{{transaction_id}}} [{destination}] Failed to parse response"
|
||||
);
|
||||
return Err(ClientError::SendFailed {
|
||||
cause: FailureCause::InvalidBody(format!(
|
||||
@@ -270,7 +293,7 @@ impl TransactionSender {
|
||||
can_retry: false,
|
||||
});
|
||||
};
|
||||
log::info!("{log_prefix} got 200 response");
|
||||
log::info!(target: TX_LOG, "{log_prefix} got 200 response");
|
||||
Ok(TransactionResponse::Acknowledged(acknowledged_pdus(
|
||||
&log_prefix,
|
||||
&value,
|
||||
@@ -292,7 +315,10 @@ impl TransactionSender {
|
||||
let phrase = response.phrase.clone();
|
||||
|
||||
if let Err(err) = check_content_type(&response.headers, "application/json") {
|
||||
log::warn!("{log_prefix} Error reading response {description}: {err}");
|
||||
log::warn!(
|
||||
target: CLIENT_LOG,
|
||||
"{log_prefix} Error reading response {description}: {err}"
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
@@ -317,13 +343,20 @@ impl TransactionSender {
|
||||
cause: FailureCause::TooLarge { limit },
|
||||
..
|
||||
} => log::warn!(
|
||||
target: CLIENT_LOG,
|
||||
"{log_prefix} JSON response exceeded max size {limit} - {description}"
|
||||
),
|
||||
ClientError::SendFailed {
|
||||
cause: FailureCause::BodyTimeout,
|
||||
..
|
||||
} => log::warn!("{log_prefix} Timed out reading response - {description}"),
|
||||
_ => log::warn!("{log_prefix} Failed to read response - {description}"),
|
||||
} => log::warn!(
|
||||
target: CLIENT_LOG,
|
||||
"{log_prefix} Timed out reading response - {description}"
|
||||
),
|
||||
_ => log::warn!(
|
||||
target: CLIENT_LOG,
|
||||
"{log_prefix} Failed to read response - {description}"
|
||||
),
|
||||
}
|
||||
return Err(err);
|
||||
}
|
||||
@@ -331,6 +364,7 @@ impl TransactionSender {
|
||||
|
||||
let secs = self.client.now_ms().saturating_sub(start_ms) as f64 / 1000.0;
|
||||
log::info!(
|
||||
target: CLIENT_LOG,
|
||||
"{log_prefix} Completed request: {status} {phrase} in {secs:.2} secs, got {length} bytes - {description}"
|
||||
);
|
||||
Ok(collected.freeze())
|
||||
|
||||
@@ -25,7 +25,10 @@ use std::sync::Arc;
|
||||
use pyo3::{intern, prelude::*, types::PyDict};
|
||||
use serde_json::Value;
|
||||
|
||||
use super::{acknowledged_pdus, Transaction, TransactionResponse, TransactionSender};
|
||||
use super::{
|
||||
acknowledged_pdus, log_prefix, Transaction, TransactionResponse, TransactionSender, CLIENT_LOG,
|
||||
TX_LOG,
|
||||
};
|
||||
use crate::{
|
||||
deferred::{create_deferred_with, OnCancel},
|
||||
events::Event,
|
||||
@@ -133,7 +136,7 @@ impl RustTransactionManager {
|
||||
// GIL.
|
||||
let pdus = pdus
|
||||
.iter()
|
||||
.map(|pdu| pdu.get().to_pdu_value(None))
|
||||
.map(|pdu| pdu.get().to_pdu_value())
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.map_err(|err| {
|
||||
pyo3::exceptions::PyValueError::new_err(format!(
|
||||
@@ -151,8 +154,9 @@ impl RustTransactionManager {
|
||||
})?;
|
||||
}
|
||||
|
||||
let transaction_id = transaction.transaction_id.clone();
|
||||
let mut stats = RequestStats::default();
|
||||
let result = sender.send(&transaction, extra_headers, &mut stats).await;
|
||||
let result = sender.send(transaction, extra_headers, &mut stats).await;
|
||||
|
||||
with_gil("transaction_response", |py| -> PyResult<Py<PyAny>> {
|
||||
let recorded =
|
||||
@@ -166,7 +170,7 @@ impl RustTransactionManager {
|
||||
let acknowledged = match result {
|
||||
Ok(TransactionResponse::Acknowledged(ids)) => ids,
|
||||
Ok(TransactionResponse::Unparsed(body)) => {
|
||||
python_acknowledged(py, &transaction, &body)?
|
||||
python_acknowledged(py, &destination, &transaction_id, &body)?
|
||||
}
|
||||
Err(err) => return Err(client_error_to_py(py, err, &destination)),
|
||||
};
|
||||
@@ -192,7 +196,7 @@ impl RustTransactionManager {
|
||||
) -> PyResult<RustTransaction> {
|
||||
let pdus = pdus
|
||||
.iter()
|
||||
.map(|pdu| pdu.to_pdu_value(None))
|
||||
.map(|pdu| pdu.to_pdu_value())
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.map_err(|err| {
|
||||
pyo3::exceptions::PyValueError::new_err(format!("Failed to serialize event: {err}"))
|
||||
@@ -229,15 +233,15 @@ impl RustTransactionManager {
|
||||
/// result.
|
||||
fn python_acknowledged(
|
||||
py: Python<'_>,
|
||||
transaction: &Transaction,
|
||||
destination: &str,
|
||||
transaction_id: &str,
|
||||
body: &[u8],
|
||||
) -> PyResult<Vec<String>> {
|
||||
let log_prefix = transaction.log_prefix();
|
||||
let log_prefix = log_prefix(destination, transaction_id);
|
||||
let failed = |err: PyErr| -> PyErr {
|
||||
log::warn!(
|
||||
"{{{}}} [{}] Failed to parse response",
|
||||
transaction.transaction_id,
|
||||
transaction.destination
|
||||
target: CLIENT_LOG,
|
||||
"{{{transaction_id}}} [{destination}] Failed to parse response"
|
||||
);
|
||||
request_send_failed(py, err.into_value(py).into_bound(py).into_any(), false)
|
||||
};
|
||||
@@ -262,13 +266,13 @@ fn python_acknowledged(
|
||||
"Received incorrect JSON value: {name}"
|
||||
))));
|
||||
};
|
||||
log::info!("{log_prefix} got 200 response");
|
||||
log::info!(target: TX_LOG, "{log_prefix} got 200 response");
|
||||
|
||||
let Some(pdus) = response.get_item("pdus")? else {
|
||||
return Ok(Vec::new());
|
||||
};
|
||||
let Ok(pdus) = pdus.cast::<PyDict>() else {
|
||||
log::warn!("{log_prefix} Remote returned invalid type for `pdus`");
|
||||
log::warn!(target: TX_LOG, "{log_prefix} Remote returned invalid type for `pdus`");
|
||||
return Ok(Vec::new());
|
||||
};
|
||||
let mut acknowledged = Vec::with_capacity(pdus.len());
|
||||
@@ -280,7 +284,10 @@ fn python_acknowledged(
|
||||
// An id Rust cannot hold is not one of ours.
|
||||
match (accepted, event_id.extract::<String>()) {
|
||||
(true, Ok(event_id)) => acknowledged.push(event_id),
|
||||
_ => log::warn!("{log_prefix} Remote returned error for {event_id}: {result}"),
|
||||
_ => log::warn!(
|
||||
target: TX_LOG,
|
||||
"{log_prefix} Remote returned error for {event_id}: {result}"
|
||||
),
|
||||
}
|
||||
}
|
||||
Ok(acknowledged)
|
||||
|
||||
@@ -120,7 +120,7 @@ async fn send_puts_the_body() {
|
||||
|
||||
let mut stats = RequestStats::default();
|
||||
let response = sender
|
||||
.send(&transaction, Vec::new(), &mut stats)
|
||||
.send(transaction.clone(), Vec::new(), &mut stats)
|
||||
.await
|
||||
.unwrap();
|
||||
let TransactionResponse::Acknowledged(acked) = response else {
|
||||
@@ -155,7 +155,7 @@ async fn send_retries_server_errors() {
|
||||
|
||||
let mut stats = RequestStats::default();
|
||||
sender
|
||||
.send(&transaction, Vec::new(), &mut stats)
|
||||
.send(transaction.clone(), Vec::new(), &mut stats)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(stats.attempts, 2);
|
||||
@@ -169,7 +169,11 @@ async fn send_fails_on_client_errors() {
|
||||
let transaction = sender.build(DEST, vec![], vec![]);
|
||||
|
||||
let err = sender
|
||||
.send(&transaction, Vec::new(), &mut RequestStats::default())
|
||||
.send(
|
||||
transaction.clone(),
|
||||
Vec::new(),
|
||||
&mut RequestStats::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
@@ -185,7 +189,11 @@ async fn send_rejects_a_non_object() {
|
||||
let transaction = sender.build(DEST, vec![], vec![]);
|
||||
|
||||
let err = sender
|
||||
.send(&transaction, Vec::new(), &mut RequestStats::default())
|
||||
.send(
|
||||
transaction.clone(),
|
||||
Vec::new(),
|
||||
&mut RequestStats::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
@@ -207,7 +215,11 @@ async fn send_hands_back_what_serde_refuses() {
|
||||
let transaction = sender.build(DEST, vec![], vec![]);
|
||||
|
||||
let response = sender
|
||||
.send(&transaction, Vec::new(), &mut RequestStats::default())
|
||||
.send(
|
||||
transaction.clone(),
|
||||
Vec::new(),
|
||||
&mut RequestStats::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(matches!(response, TransactionResponse::Unparsed(_)));
|
||||
|
||||
@@ -38,7 +38,11 @@ from synapse.logging.opentracing import (
|
||||
)
|
||||
from synapse.metrics import SERVER_NAME_LABEL
|
||||
from synapse.module_api.callbacks.federation import FederatedEventDeliveryMethod
|
||||
from synapse.synapse_rust.federation import RustTransaction, RustTransactionManager
|
||||
from synapse.synapse_rust.federation import (
|
||||
RustFederationClient,
|
||||
RustTransaction,
|
||||
RustTransactionManager,
|
||||
)
|
||||
from synapse.types import JsonDict
|
||||
from synapse.util.json import json_decoder
|
||||
from synapse.util.metrics import measure_func
|
||||
@@ -66,10 +70,6 @@ class TransactionManager:
|
||||
callback.
|
||||
"""
|
||||
|
||||
# For tests: check that each transaction Rust sends has the body Python
|
||||
# would have sent.
|
||||
_rust_strict = False
|
||||
|
||||
def __init__(self, hs: "synapse.server.HomeServer"):
|
||||
self.server_name = hs.hostname # nb must be called this for @measure_func
|
||||
self.clock = hs.get_clock() # nb must be called this for @measure_func
|
||||
@@ -93,10 +93,16 @@ class TransactionManager:
|
||||
self._rust: RustTransactionManager | None = None
|
||||
self._is_mine_server_name = hs.is_mine_server_name
|
||||
if hs.config.experimental.rust_federation_sending_at_least("transactions"):
|
||||
# Tests may replace the federation client with a mock, which has
|
||||
# no Rust client to send through.
|
||||
rust_client = getattr(hs.get_federation_http_client(), "_rust_client", None)
|
||||
if rust_client is not None:
|
||||
if isinstance(rust_client, RustFederationClient):
|
||||
self._rust = RustTransactionManager(rust_client)
|
||||
|
||||
# For tests: check that each transaction Rust sends has the body Python
|
||||
# would have sent.
|
||||
self._rust_strict = False
|
||||
|
||||
def shutdown(self) -> None:
|
||||
self._is_shutdown = True
|
||||
self._transport_layer.shutdown()
|
||||
|
||||
@@ -31,6 +31,8 @@ from synapse.synapse_rust.federation import RustTransaction, RustTransactionMana
|
||||
from synapse.types import JsonDict
|
||||
from synapse.util.clock import Clock
|
||||
|
||||
from tests.utils import RUST_FED_STRICT_FOR_TESTS
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from unittest.mock import Mock
|
||||
|
||||
@@ -94,6 +96,7 @@ class RustTransactionsMixin(_Base):
|
||||
assert isinstance(sender, FederationSender)
|
||||
manager = sender._transaction_manager
|
||||
assert manager._rust is not None
|
||||
manager._rust_strict = RUST_FED_STRICT_FOR_TESTS
|
||||
manager._rust = _MockedRustTransactions( # type: ignore[assignment]
|
||||
manager._rust, self.federation_transport_client
|
||||
)
|
||||
@@ -22,7 +22,7 @@ Python builds.
|
||||
|
||||
import json
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
|
||||
from twisted.internet.testing import MemoryReactor
|
||||
|
||||
@@ -41,6 +41,7 @@ from synapse.module_api.callbacks.federation import (
|
||||
from synapse.rest import admin
|
||||
from synapse.rest.client import login, room
|
||||
from synapse.server import HomeServer
|
||||
from synapse.synapse_rust import reset_logging_config
|
||||
from synapse.synapse_rust.federation import (
|
||||
RustFederationClient,
|
||||
RustTransactionManager,
|
||||
@@ -54,6 +55,7 @@ from tests.http.test_rust_federation_client import (
|
||||
FakeTransport,
|
||||
json_response,
|
||||
)
|
||||
from tests.utils import RUST_FED_STRICT_FOR_TESTS
|
||||
|
||||
|
||||
class RustTransactionsTestCase(unittest.HomeserverTestCase):
|
||||
@@ -85,6 +87,7 @@ class RustTransactionsTestCase(unittest.HomeserverTestCase):
|
||||
|
||||
self.manager = TransactionManager(hs)
|
||||
assert self.manager._rust is not None
|
||||
self.manager._rust_strict = RUST_FED_STRICT_FOR_TESTS
|
||||
self.manager._rust = RustTransactionManager(rust_client)
|
||||
|
||||
user = self.register_user("user", "pass")
|
||||
@@ -122,6 +125,37 @@ class RustTransactionsTestCase(unittest.HomeserverTestCase):
|
||||
self.assertEqual(body["edus"], [edu.get_dict()])
|
||||
self.assertEqual(body["pdus"], [self.event.get_pdu_json()])
|
||||
|
||||
def test_tracing_headers(self) -> None:
|
||||
"""The headers of the tracing span reach the request."""
|
||||
|
||||
def inject(headers: dict[bytes, list[bytes]], destination: str) -> None:
|
||||
headers[b"uber-trace-id"] = [b"abc:def:0:1"]
|
||||
|
||||
self.transport.responses.append(json_response(200, {}))
|
||||
with patch(
|
||||
"synapse.federation.sender.transaction_manager.opentracing.inject_header_dict",
|
||||
side_effect=inject,
|
||||
):
|
||||
self._send([self.event])
|
||||
|
||||
[request] = self.transport.requests
|
||||
self.assertEqual(request.header(b"uber-trace-id"), b"abc:def:0:1")
|
||||
|
||||
def test_log_lines(self) -> None:
|
||||
"""The `TX` lines are logged under the Python transaction manager's
|
||||
logger name, so log configs that name it still apply."""
|
||||
self.transport.responses.append(json_response(200, {}))
|
||||
with self.assertLogs(
|
||||
"synapse.federation.sender.transaction_manager", level="INFO"
|
||||
) as logs:
|
||||
# pyo3_log caches levels. Make it see the level assertLogs set.
|
||||
reset_logging_config()
|
||||
self._send([self.event])
|
||||
|
||||
messages = [record.getMessage() for record in logs.records]
|
||||
self.assertTrue(any("Sending transaction" in m for m in messages), messages)
|
||||
self.assertTrue(any("got 200 response" in m for m in messages), messages)
|
||||
|
||||
def test_txn_ids_increase(self) -> None:
|
||||
self.transport.responses.extend([json_response(200, {})] * 2)
|
||||
self._send([self.event])
|
||||
@@ -289,3 +323,19 @@ class RustTransactionsLevelTestCase(unittest.HomeserverTestCase):
|
||||
"""An instance that sends through a proxy worker has no Rust client,
|
||||
so it builds transactions in Python."""
|
||||
self.assertIsNone(self._manager()._rust)
|
||||
|
||||
|
||||
class MockedHttpClientTestCase(unittest.HomeserverTestCase):
|
||||
"""A test that mocks the federation client gets the Python transaction
|
||||
builder at any level."""
|
||||
|
||||
def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
|
||||
return self.setup_test_homeserver(federation_http_client=Mock())
|
||||
|
||||
def default_config(self) -> dict[str, Any]:
|
||||
config = super().default_config()
|
||||
config["experimental_features"] = {"rust_federation_sending": "transactions"}
|
||||
return config
|
||||
|
||||
def test_no_rust_sender(self) -> None:
|
||||
self.assertIsNone(TransactionManager(self.hs)._rust)
|
||||
@@ -89,7 +89,6 @@ from synapse.config.database import DatabaseConnectionConfig
|
||||
from synapse.config.homeserver import HomeServerConfig
|
||||
from synapse.events.auto_accept_invites import InviteAutoAccepter
|
||||
from synapse.events.presence_router import load_legacy_presence_router
|
||||
from synapse.federation.sender.transaction_manager import TransactionManager
|
||||
from synapse.handlers.auth import load_legacy_password_auth_providers
|
||||
from synapse.http.site import SynapseRequest
|
||||
from synapse.logging.context import ContextResourceUsage
|
||||
@@ -124,7 +123,6 @@ from tests.utils import (
|
||||
POSTGRES_PASSWORD,
|
||||
POSTGRES_PORT,
|
||||
POSTGRES_USER,
|
||||
RUST_FED_STRICT_FOR_TESTS,
|
||||
RUST_STATE_RES_FOR_TESTS,
|
||||
RUST_SYNC_FOR_TESTS,
|
||||
RUST_SYNC_STRICT_FOR_TESTS,
|
||||
@@ -136,8 +134,6 @@ from tests.utils import (
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
TransactionManager._rust_strict = RUST_FED_STRICT_FOR_TESTS
|
||||
|
||||
R = TypeVar("R")
|
||||
P = ParamSpec("P")
|
||||
|
||||
|
||||
Reference in new issue
Block a user