Skip to content

MatrixRTC: Rename MembershipConfig parameters #4714

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

Merged
merged 19 commits into from
May 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions spec/unit/matrixrtc/MatrixRTCSession.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,10 +402,10 @@ describe("MatrixRTCSession", () => {
jest.useRealTimers();
});

it("uses membershipExpiryTimeout from join config", async () => {
it("uses membershipEventExpiryMs from join config", async () => {
const realSetTimeout = setTimeout;
jest.useFakeTimers();
sess!.joinRoomSession([mockFocus], mockFocus, { membershipExpiryTimeout: 60000 });
sess!.joinRoomSession([mockFocus], mockFocus, { membershipEventExpiryMs: 60000 });
await Promise.race([sentStateEvent, new Promise((resolve) => realSetTimeout(resolve, 500))]);
expect(client.sendStateEvent).toHaveBeenCalledWith(
mockRoom!.roomId,
Expand Down
18 changes: 9 additions & 9 deletions spec/unit/matrixrtc/MembershipManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ describe.each([
});
const manager = new TestMembershipManager(
{
membershipServerSideExpiryTimeout: 9000,
delayedLeaveEventDelayMs: 9000,
},
room,
client,
Expand Down Expand Up @@ -294,9 +294,9 @@ describe.each([
await jest.advanceTimersByTimeAsync(5000);
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(2);
});
it("uses membershipServerSideExpiryTimeout from config", () => {
it("uses delayedLeaveEventDelayMs from config", () => {
const manager = new TestMembershipManager(
{ membershipServerSideExpiryTimeout: 123456 },
{ delayedLeaveEventDelayMs: 123456 },
room,
client,
() => undefined,
Expand All @@ -312,9 +312,9 @@ describe.each([
});
});

it("uses membershipExpiryTimeout from config", async () => {
it("uses membershipEventExpiryMs from config", async () => {
const manager = new TestMembershipManager(
{ membershipExpiryTimeout: 1234567 },
{ membershipEventExpiryMs: 1234567 },
room,
client,
() => undefined,
Expand Down Expand Up @@ -480,9 +480,9 @@ describe.each([

// TODO: Not sure about this name
describe("background timers", () => {
it("sends only one keep-alive for delayed leave event per `membershipKeepAlivePeriod`", async () => {
it("sends only one keep-alive for delayed leave event per `delayedLeaveEventRestartMs`", async () => {
const manager = new TestMembershipManager(
{ membershipKeepAlivePeriod: 10_000, membershipServerSideExpiryTimeout: 30_000 },
{ delayedLeaveEventRestartMs: 10_000, delayedLeaveEventDelayMs: 30_000 },
room,
client,
() => undefined,
Expand Down Expand Up @@ -513,7 +513,7 @@ describe.each([
// TODO: Add git commit when we removed it.
async function testExpires(expire: number, headroom?: number) {
const manager = new TestMembershipManager(
{ membershipExpiryTimeout: expire, membershipExpiryTimeoutHeadroom: headroom },
{ membershipEventExpiryMs: expire, membershipEventExpiryHeadroomMs: headroom },
room,
client,
() => undefined,
Expand Down Expand Up @@ -734,7 +734,7 @@ describe.each([
const unrecoverableError = jest.fn();
(client._unstable_sendDelayedStateEvent as Mock<any>).mockRejectedValue(new HTTPError("unknown", 501));
const manager = new TestMembershipManager(
{ callMemberEventRetryDelayMinimum: 1000, maximumNetworkErrorRetryCount: 7 },
{ networkErrorRetryMs: 1000, maximumNetworkErrorRetryCount: 7 },
room,
client,
() => undefined,
Expand Down
40 changes: 21 additions & 19 deletions src/matrixrtc/LegacyMembershipManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,32 @@ export class LegacyMembershipManager implements IMembershipManager {
private updateCallMembershipRunning = false;
private needCallMembershipUpdate = false;
/**
* If the server disallows the configured {@link membershipServerSideExpiryTimeout},
* If the server disallows the configured {@link delayedLeaveEventDelayMs},
* this stores a delay that the server does allow.
*/
private membershipServerSideExpiryTimeoutOverride?: number;
private delayedLeaveEventDelayMsOverride?: number;
private disconnectDelayId: string | undefined;

private get callMemberEventRetryDelayMinimum(): number {
return this.joinConfig?.callMemberEventRetryDelayMinimum ?? 3_000;
private get networkErrorRetryMs(): number {
return this.joinConfig?.networkErrorRetryMs ?? this.joinConfig?.callMemberEventRetryDelayMinimum ?? 3_000;
}
private get membershipExpiryTimeout(): number {
return this.joinConfig?.membershipExpiryTimeout ?? DEFAULT_EXPIRE_DURATION;
private get membershipEventExpiryMs(): number {
return (
this.joinConfig?.membershipEventExpiryMs ??
this.joinConfig?.membershipExpiryTimeout ??
DEFAULT_EXPIRE_DURATION
);
}
private get membershipServerSideExpiryTimeout(): number {
private get delayedLeaveEventDelayMs(): number {
return (
this.membershipServerSideExpiryTimeoutOverride ??
this.delayedLeaveEventDelayMsOverride ??
this.joinConfig?.delayedLeaveEventDelayMs ??
this.joinConfig?.membershipServerSideExpiryTimeout ??
8_000
);
}
private get membershipKeepAlivePeriod(): number {
return this.joinConfig?.membershipKeepAlivePeriod ?? 5_000;
}
private get callMemberEventRetryJitter(): number {
return this.joinConfig?.callMemberEventRetryJitter ?? 2_000;
private get delayedLeaveEventRestartMs(): number {
return this.joinConfig?.delayedLeaveEventRestartMs ?? this.joinConfig?.membershipKeepAlivePeriod ?? 5_000;
}

public constructor(
Expand Down Expand Up @@ -137,7 +139,7 @@ export class LegacyMembershipManager implements IMembershipManager {
public join(fociPreferred: Focus[], fociActive?: Focus): void {
this.ownFocusActive = fociActive;
this.ownFociPreferred = fociPreferred;
this.relativeExpiry = this.membershipExpiryTimeout;
this.relativeExpiry = this.membershipEventExpiryMs;
// We don't wait for this, mostly because it may fail and schedule a retry, so this
// function returning doesn't really mean anything at all.
void this.triggerCallMembershipEventUpdate();
Expand Down Expand Up @@ -261,7 +263,7 @@ export class LegacyMembershipManager implements IMembershipManager {
this.client._unstable_sendDelayedStateEvent(
this.room.roomId,
{
delay: this.membershipServerSideExpiryTimeout,
delay: this.delayedLeaveEventDelayMs,
},
EventType.GroupCallMemberPrefix,
{}, // leave event
Expand All @@ -278,9 +280,9 @@ export class LegacyMembershipManager implements IMembershipManager {
const maxDelayAllowed = e.data["org.matrix.msc4140.max_delay"];
if (
typeof maxDelayAllowed === "number" &&
this.membershipServerSideExpiryTimeout > maxDelayAllowed
this.delayedLeaveEventDelayMs > maxDelayAllowed
) {
this.membershipServerSideExpiryTimeoutOverride = maxDelayAllowed;
this.delayedLeaveEventDelayMsOverride = maxDelayAllowed;
return prepareDelayedDisconnection();
}
}
Expand Down Expand Up @@ -350,15 +352,15 @@ export class LegacyMembershipManager implements IMembershipManager {
}
logger.info("Sent updated call member event.");
} catch (e) {
const resendDelay = this.callMemberEventRetryDelayMinimum + Math.random() * this.callMemberEventRetryJitter;
const resendDelay = this.networkErrorRetryMs;
logger.warn(`Failed to send call member event (retrying in ${resendDelay}): ${e}`);
await sleep(resendDelay);
await this.triggerCallMembershipEventUpdate();
}
}

private scheduleDelayDisconnection(): void {
this.memberEventTimeout = setTimeout(() => void this.delayDisconnection(), this.membershipKeepAlivePeriod);
this.memberEventTimeout = setTimeout(() => void this.delayDisconnection(), this.delayedLeaveEventRestartMs);
}

private readonly delayDisconnection = async (): Promise<void> => {
Expand Down
41 changes: 23 additions & 18 deletions src/matrixrtc/MatrixRTCSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ export type MatrixRTCSessionEventHandlerMap = {
) => void;
[MatrixRTCSessionEvent.MembershipManagerError]: (error: unknown) => void;
};

// The names follow these principles:
// - we use the technical term delay if the option is related to delayed events.
// - we use delayedLeaveEvent if the option is related to the delayed leave event.
// - we use membershipEvent if the option is related to the rtc member state event.
// - we use the technical term expiry if the option is related to the expiry field of the membership state event.
// - we use a `MS` postfix if the option is a duration to avoid using words like:
// `time`, `duration`, `delay`, `timeout`... that might be mistaken/confused with technical terms.
export interface MembershipConfig {
/**
* Use the new Manager.
Expand All @@ -74,6 +80,8 @@ export interface MembershipConfig {
*
* This is what goes into the m.rtc.member event expiry field and is typically set to a number of hours.
*/
membershipEventExpiryMs?: number;
/** @deprecated renamed to `membershipEventExpiryMs`*/
membershipExpiryTimeout?: number;

/**
Expand All @@ -85,36 +93,25 @@ export interface MembershipConfig {
*
* This value does not have an effect on the value of `SessionMembershipData.expires`.
*/
membershipEventExpiryHeadroomMs?: number;
/** @deprecated renamed to `membershipEventExpiryHeadroomMs`*/
membershipExpiryTimeoutHeadroom?: number;

/**
* The period (in milliseconds) with which we check that our membership event still exists on the
* server. If it is not found we create it again.
*/
memberEventCheckPeriod?: number;

/**
* The minimum delay (in milliseconds) after which we will retry sending the membership event if it
* failed to send.
*/
callMemberEventRetryDelayMinimum?: number;

/**
* The timeout (in milliseconds) with which the deleayed leave event on the server is configured.
* After this time the server will set the event to the disconnected stat if it has not received a keep-alive from the client.
*/
delayedLeaveEventDelayMs?: number;
/** @deprecated renamed to `delayedLeaveEventDelayMs`*/
membershipServerSideExpiryTimeout?: number;

/**
* The interval (in milliseconds) in which the client will send membership keep-alives to the server.
*/
delayedLeaveEventRestartMs?: number;
/** @deprecated renamed to `delayedLeaveEventRestartMs`*/
membershipKeepAlivePeriod?: number;

/**
* @deprecated It should be possible to make it stable without this.
*/
callMemberEventRetryJitter?: number;

/**
* The maximum number of retries that the manager will do for delayed event sending/updating and state event sending when a server rate limit has been hit.
*/
Expand All @@ -125,6 +122,14 @@ export interface MembershipConfig {
*/
maximumNetworkErrorRetryCount?: number;

/**
* The time (in milliseconds) after which we will retry a http request if it
* failed to send due to a network error. (send membership event, send delayed event, restart delayed event...)
*/
networkErrorRetryMs?: number;
/** @deprecated renamed to `networkErrorRetryMs`*/
callMemberEventRetryDelayMinimum?: number;

/**
* If true, use the new to-device transport for sending encryption keys.
*/
Expand Down
55 changes: 30 additions & 25 deletions src/matrixrtc/NewMembershipManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,33 +333,38 @@ export class MembershipManager
private focusActive?: Focus;

// Config:
private membershipServerSideExpiryTimeoutOverride?: number;
private delayedLeaveEventDelayMsOverride?: number;

private get callMemberEventRetryDelayMinimum(): number {
return this.joinConfig?.callMemberEventRetryDelayMinimum ?? 3_000;
private get networkErrorRetryMs(): number {
return this.joinConfig?.networkErrorRetryMs ?? this.joinConfig?.callMemberEventRetryDelayMinimum ?? 3_000;
}
private get membershipEventExpiryTimeout(): number {
return this.joinConfig?.membershipExpiryTimeout ?? DEFAULT_EXPIRE_DURATION;
}
private get membershipEventExpiryTimeoutHeadroom(): number {
return this.joinConfig?.membershipExpiryTimeoutHeadroom ?? 5_000;
private get membershipEventExpiryMs(): number {
return (
this.joinConfig?.membershipEventExpiryMs ??
this.joinConfig?.membershipExpiryTimeout ??
DEFAULT_EXPIRE_DURATION
);
}
private computeNextExpiryActionTs(iteration: number): number {
private get membershipEventExpiryHeadroomMs(): number {
return (
this.state.startTime +
this.membershipEventExpiryTimeout * iteration -
this.membershipEventExpiryTimeoutHeadroom
this.joinConfig?.membershipEventExpiryHeadroomMs ??
this.joinConfig?.membershipExpiryTimeoutHeadroom ??
5_000
);
}
private get membershipServerSideExpiryTimeout(): number {
private computeNextExpiryActionTs(iteration: number): number {
return this.state.startTime + this.membershipEventExpiryMs * iteration - this.membershipEventExpiryHeadroomMs;
}
private get delayedLeaveEventDelayMs(): number {
return (
this.membershipServerSideExpiryTimeoutOverride ??
this.delayedLeaveEventDelayMsOverride ??
this.joinConfig?.delayedLeaveEventDelayMs ??
this.joinConfig?.membershipServerSideExpiryTimeout ??
8_000
);
}
private get membershipKeepAlivePeriod(): number {
return this.joinConfig?.membershipKeepAlivePeriod ?? 5_000;
private get delayedLeaveEventRestartMs(): number {
return this.joinConfig?.delayedLeaveEventRestartMs ?? this.joinConfig?.membershipKeepAlivePeriod ?? 5_000;
}
private get maximumRateLimitRetryCount(): number {
return this.joinConfig?.maximumRateLimitRetryCount ?? 10;
Expand Down Expand Up @@ -432,7 +437,7 @@ export class MembershipManager
._unstable_sendDelayedStateEvent(
this.room.roomId,
{
delay: this.membershipServerSideExpiryTimeout,
delay: this.delayedLeaveEventDelayMs,
},
EventType.GroupCallMemberPrefix,
{}, // leave event
Expand All @@ -447,7 +452,7 @@ export class MembershipManager
// due to lack of https://github.com/element-hq/synapse/pull/17810
return createInsertActionUpdate(
MembershipActionType.RestartDelayedEvent,
this.membershipKeepAlivePeriod,
this.delayedLeaveEventRestartMs,
);
} else {
// This action was scheduled because we are in the process of joining
Expand Down Expand Up @@ -525,7 +530,7 @@ export class MembershipManager
this.resetRateLimitCounter(MembershipActionType.RestartDelayedEvent);
return createInsertActionUpdate(
MembershipActionType.RestartDelayedEvent,
this.membershipKeepAlivePeriod,
this.delayedLeaveEventRestartMs,
);
})
.catch((e) => {
Expand Down Expand Up @@ -579,7 +584,7 @@ export class MembershipManager
.sendStateEvent(
this.room.roomId,
EventType.GroupCallMemberPrefix,
this.makeMyMembership(this.membershipEventExpiryTimeout),
this.makeMyMembership(this.membershipEventExpiryMs),
this.stateKey,
)
.then(() => {
Expand Down Expand Up @@ -611,7 +616,7 @@ export class MembershipManager
.sendStateEvent(
this.room.roomId,
EventType.GroupCallMemberPrefix,
this.makeMyMembership(this.membershipEventExpiryTimeout * nextExpireUpdateIteration),
this.makeMyMembership(this.membershipEventExpiryMs * nextExpireUpdateIteration),
this.stateKey,
)
.then(() => {
Expand Down Expand Up @@ -697,8 +702,8 @@ export class MembershipManager
error.data["org.matrix.msc4140.errcode"] === "M_MAX_DELAY_EXCEEDED"
) {
const maxDelayAllowed = error.data["org.matrix.msc4140.max_delay"];
if (typeof maxDelayAllowed === "number" && this.membershipServerSideExpiryTimeout > maxDelayAllowed) {
this.membershipServerSideExpiryTimeoutOverride = maxDelayAllowed;
if (typeof maxDelayAllowed === "number" && this.delayedLeaveEventDelayMs > maxDelayAllowed) {
this.delayedLeaveEventDelayMsOverride = maxDelayAllowed;
}
this.logger.warn("Retry sending delayed disconnection event due to server timeout limitations:", error);
return true;
Expand Down Expand Up @@ -769,7 +774,7 @@ export class MembershipManager
private actionUpdateFromNetworkErrorRetry(error: unknown, type: MembershipActionType): ActionUpdate | undefined {
// "Is a network error"-boundary
const retries = this.state.networkErrorRetries.get(type) ?? 0;
const retryDurationString = this.callMemberEventRetryDelayMinimum / 1000 + "s";
const retryDurationString = this.networkErrorRetryMs / 1000 + "s";
const retryCounterString = "(" + retries + "/" + this.maximumNetworkErrorRetryCount + ")";
if (error instanceof Error && error.name === "AbortError") {
this.logger.warn(
Expand Down Expand Up @@ -819,7 +824,7 @@ export class MembershipManager
// retry boundary
if (retries < this.maximumNetworkErrorRetryCount) {
this.state.networkErrorRetries.set(type, retries + 1);
return createInsertActionUpdate(type, this.callMemberEventRetryDelayMinimum);
return createInsertActionUpdate(type, this.networkErrorRetryMs);
}

// Failure
Expand Down