Skip to content

Commit 31c5cfd

Browse files
hughnshughns
committed
Suggestions from pair review
1 parent de492d9 commit 31c5cfd

2 files changed

Lines changed: 50 additions & 38 deletions

File tree

‎src/matrixrtc/NewMembershipManager.ts‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ export class MembershipManager implements IMembershipManager {
345345
case MembershipActionType.RestartDelayedEvent: {
346346
if (!state.delayId) {
347347
// Delay id got reset. This action was used to check if the hs canceled the delayed event when the join state got sent.
348-
return createAddActionUpdate(
348+
return createInsertActionUpdate(
349349
state.hasMemberStateEvent
350350
? MembershipActionType.SendMainDelayedEvent
351351
: MembershipActionType.SendFirstDelayedEvent,
@@ -361,12 +361,12 @@ export class MembershipManager implements IMembershipManager {
361361
if (!state.hasMemberStateEvent) {
362362
this.leavePromiseDefer?.resolve(true);
363363
this.leavePromiseDefer = undefined;
364-
return { setActions: [] };
364+
return { replace: [] };
365365
}
366366
if (state.delayId) {
367367
return this.sendScheduledDelayedLeaveEventOrFallbackToSendLeaveEvent(state, type, state.delayId);
368368
} else {
369-
return createAddActionUpdate(MembershipActionType.SendLeaveEvent);
369+
return createInsertActionUpdate(MembershipActionType.SendLeaveEvent);
370370
}
371371
}
372372
case MembershipActionType.SendJoinEvent: {
@@ -380,7 +380,7 @@ export class MembershipManager implements IMembershipManager {
380380
if (!state.hasMemberStateEvent) {
381381
this.leavePromiseDefer?.resolve(true);
382382
this.leavePromiseDefer = undefined;
383-
return { setActions: [] };
383+
return { replace: [] };
384384
}
385385
// This is only a fallback in case we do not have working delayed events support.
386386
// first we should try to just send the scheduled leave event
@@ -410,12 +410,12 @@ export class MembershipManager implements IMembershipManager {
410410
state.rateLimitRetries.set(MembershipActionType.SendFirstDelayedEvent, 0);
411411
state.networkErrorRetries.set(MembershipActionType.SendFirstDelayedEvent, 0);
412412
state.delayId = response.delay_id;
413-
return createAddActionUpdate(MembershipActionType.SendJoinEvent);
413+
return createInsertActionUpdate(MembershipActionType.SendJoinEvent);
414414
})
415415
.catch((e) => {
416416
if (this.manageMaxDelayExceededSituation(e)) {
417417
return {
418-
addActions: [
418+
insert: [
419419
{
420420
ts: Date.now(),
421421
type: MembershipActionType.SendFirstDelayedEvent,
@@ -435,7 +435,7 @@ export class MembershipManager implements IMembershipManager {
435435
logger.info("Not using delayed event because: " + e);
436436
}
437437
// On any other error we fall back to not using delayed events and send the join state event immediately
438-
return createAddActionUpdate(MembershipActionType.SendJoinEvent);
438+
return createInsertActionUpdate(MembershipActionType.SendJoinEvent);
439439
});
440440
}
441441

@@ -445,15 +445,15 @@ export class MembershipManager implements IMembershipManager {
445445
delayId: string,
446446
): Promise<ActionUpdate> {
447447
// Remove all running updates and restarts
448-
const resetActionUpdate = { setActions: [] };
448+
const resetActionUpdate = { replace: [] };
449449
return await this.client
450450
._unstable_updateDelayedEvent(delayId, UpdateDelayedEventAction.Cancel)
451451
.then(() => {
452452
state.delayId = undefined;
453453
this.scheduler.resetRateLimitCounter(MembershipActionType.SendFirstDelayedEvent);
454454
return {
455455
...resetActionUpdate,
456-
...createAddActionUpdate(MembershipActionType.SendFirstDelayedEvent),
456+
...createInsertActionUpdate(MembershipActionType.SendFirstDelayedEvent),
457457
};
458458
})
459459
.catch((e) => {
@@ -468,13 +468,13 @@ export class MembershipManager implements IMembershipManager {
468468
state.delayId = undefined;
469469
return {
470470
...resetActionUpdate,
471-
...createAddActionUpdate(MembershipActionType.SendFirstDelayedEvent),
471+
...createInsertActionUpdate(MembershipActionType.SendFirstDelayedEvent),
472472
};
473473
}
474474
if (this.isUnsupportedDelayedEndpoint(e)) {
475475
return {
476476
...resetActionUpdate,
477-
...createAddActionUpdate(MembershipActionType.SendJoinEvent),
477+
...createInsertActionUpdate(MembershipActionType.SendJoinEvent),
478478
};
479479
}
480480

@@ -497,12 +497,15 @@ export class MembershipManager implements IMembershipManager {
497497
._unstable_updateDelayedEvent(delayId, UpdateDelayedEventAction.Restart)
498498
.then(() => {
499499
this.scheduler.resetRateLimitCounter(MembershipActionType.RestartDelayedEvent);
500-
return createAddActionUpdate(MembershipActionType.RestartDelayedEvent, this.membershipKeepAlivePeriod);
500+
return createInsertActionUpdate(
501+
MembershipActionType.RestartDelayedEvent,
502+
this.membershipKeepAlivePeriod,
503+
);
501504
})
502505
.catch((e) => {
503506
if (this.isNotFoundError(e)) {
504507
state.delayId = undefined;
505-
return createAddActionUpdate(MembershipActionType.SendMainDelayedEvent);
508+
return createInsertActionUpdate(MembershipActionType.SendMainDelayedEvent);
506509
}
507510
// If the HS does not support delayed events we wont reschedule.
508511
if (this.isUnsupportedDelayedEndpoint(e)) return {};
@@ -532,14 +535,17 @@ export class MembershipManager implements IMembershipManager {
532535
.then((response) => {
533536
state.delayId = response.delay_id;
534537
this.scheduler.resetRateLimitCounter(MembershipActionType.SendMainDelayedEvent);
535-
return createAddActionUpdate(MembershipActionType.RestartDelayedEvent, this.membershipKeepAlivePeriod);
538+
return createInsertActionUpdate(
539+
MembershipActionType.RestartDelayedEvent,
540+
this.membershipKeepAlivePeriod,
541+
);
536542
})
537543
.catch((e) => {
538544
// Don't do any other delayed event work if its not supported.
539545
if (this.isUnsupportedDelayedEndpoint(e)) return {};
540546

541547
if (this.manageMaxDelayExceededSituation(e)) {
542-
return createAddActionUpdate(MembershipActionType.SendMainDelayedEvent);
548+
return createInsertActionUpdate(MembershipActionType.SendMainDelayedEvent);
543549
}
544550
const updateLimit = this.actionUpdateFromRateLimitError(e, "sendDelayedStateEvent", type);
545551
if (updateLimit) return updateLimit;
@@ -563,13 +569,13 @@ export class MembershipManager implements IMembershipManager {
563569

564570
this.leavePromiseDefer?.resolve(true);
565571
this.leavePromiseDefer = undefined;
566-
return { setActions: [] };
572+
return { replace: [] };
567573
})
568574
.catch((e) => {
569575
if (this.isUnsupportedDelayedEndpoint(e)) return {};
570576
if (this.isNotFoundError(e)) {
571577
state.delayId = undefined;
572-
return createAddActionUpdate(MembershipActionType.SendLeaveEvent);
578+
return createInsertActionUpdate(MembershipActionType.SendLeaveEvent);
573579
}
574580
const updateLimit = this.actionUpdateFromRateLimitError(e, "updateDelayedEvent", type);
575581
if (updateLimit) return updateLimit;
@@ -581,7 +587,7 @@ export class MembershipManager implements IMembershipManager {
581587
"Encountered unexpected error during SendScheduledDelayedLeaveEvent. Falling back to SendLeaveEvent",
582588
e,
583589
);
584-
return createAddActionUpdate(MembershipActionType.SendLeaveEvent);
590+
return createInsertActionUpdate(MembershipActionType.SendLeaveEvent);
585591
});
586592
}
587593

@@ -601,7 +607,7 @@ export class MembershipManager implements IMembershipManager {
601607
state.hasMemberStateEvent = true;
602608
this.scheduler.resetRateLimitCounter(MembershipActionType.SendJoinEvent);
603609
return {
604-
addActions: [
610+
insert: [
605611
{ ts: Date.now(), type: MembershipActionType.RestartDelayedEvent },
606612
{
607613
ts: this.computeNextExpiryActionTs(state.expireUpdateIterations),
@@ -636,7 +642,7 @@ export class MembershipManager implements IMembershipManager {
636642
this.scheduler.resetRateLimitCounter(MembershipActionType.UpdateExpiry);
637643
state.expireUpdateIterations = nextExpireUpdateIteration;
638644
return {
639-
addActions: [
645+
insert: [
640646
{
641647
ts: this.computeNextExpiryActionTs(nextExpireUpdateIteration),
642648
type: MembershipActionType.UpdateExpiry,
@@ -663,7 +669,7 @@ export class MembershipManager implements IMembershipManager {
663669
this.leavePromiseDefer?.resolve(true);
664670
this.leavePromiseDefer = undefined;
665671
state.hasMemberStateEvent = false;
666-
return { setActions: [] };
672+
return { replace: [] };
667673
})
668674
.catch((e) => {
669675
const updateLimit = this.actionUpdateFromRateLimitError(e, "sendStateEvent", type);
@@ -765,7 +771,7 @@ export class MembershipManager implements IMembershipManager {
765771
resendDelay = defaultMs;
766772
}
767773
this.scheduler.state.rateLimitRetries.set(type, rateLimitRetries + 1);
768-
return createAddActionUpdate(type, resendDelay);
774+
return createInsertActionUpdate(type, resendDelay);
769775
}
770776

771777
throw Error("Exceeded maximum retries for " + type + " attempts (client." + method + "): " + (error as Error));
@@ -833,10 +839,10 @@ export class MembershipManager implements IMembershipManager {
833839
// retry boundary
834840
if (retries < this.maximumNetworkErrorRetryCount) {
835841
this.scheduler.state.networkErrorRetries.set(type, retries + 1);
836-
return createAddActionUpdate(type, this.callMemberEventRetryDelayMinimum);
842+
return createInsertActionUpdate(type, this.callMemberEventRetryDelayMinimum);
837843
}
838844

839-
// Failiour
845+
// Failure
840846
throw Error(
841847
"Reached maximum (" + this.maximumNetworkErrorRetryCount + ") retries cause by: " + (error as Error),
842848
);
@@ -851,8 +857,8 @@ export class MembershipManager implements IMembershipManager {
851857
return error instanceof UnsupportedDelayedEventsEndpointError;
852858
}
853859
}
854-
function createAddActionUpdate(type: MembershipActionType, offset?: number): ActionUpdate {
860+
function createInsertActionUpdate(type: MembershipActionType, offset?: number): ActionUpdate {
855861
return {
856-
addActions: [{ ts: Date.now() + (offset ?? 0), type }],
862+
insert: [{ ts: Date.now() + (offset ?? 0), type }],
857863
};
858864
}

‎src/matrixrtc/NewMembershipManagerActionScheduler.ts‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { logger as rootLogger } from "../logger.ts";
2+
import { type EmptyObject } from "../matrix.ts";
23
import { sleep } from "../utils.ts";
34
import { MembershipActionType } from "./NewMembershipManager.ts";
45

@@ -43,10 +44,16 @@ export interface Action {
4344
type: MembershipActionType;
4445
}
4546
/** @internal */
46-
export interface ActionUpdate {
47-
setActions?: Action[];
48-
addActions?: Action[];
49-
}
47+
export type ActionUpdate =
48+
| {
49+
/** Replace all existing scheduled actions with this new array */
50+
replace: Action[];
51+
}
52+
| {
53+
/** Add these actions to the existing scheduled actions */
54+
insert: Action[];
55+
}
56+
| EmptyObject;
5057

5158
enum Status {
5259
Disconnected = "Disconnected",
@@ -149,13 +156,12 @@ export class ActionScheduler {
149156
// remove the processed action only after we are done processing
150157
this._actions.splice(0, 1);
151158
// The wakeupUpdate always wins since that is a direct external update.
152-
const { addActions, setActions } = wakeupUpdate ?? handlerResult;
159+
const actionUpdate = wakeupUpdate ?? handlerResult;
153160

154-
if (setActions) {
155-
this._actions = setActions;
156-
}
157-
if (addActions) {
158-
this._actions.push(...addActions);
161+
if ("replace" in actionUpdate) {
162+
this._actions = actionUpdate.replace;
163+
} else if ("insert" in actionUpdate) {
164+
this._actions.push(...actionUpdate.insert);
159165
}
160166

161167
logger.info(
@@ -166,10 +172,10 @@ export class ActionScheduler {
166172
}
167173

168174
public initiateJoin(): void {
169-
this.wakeup?.({ setActions: [{ ts: Date.now(), type: MembershipActionType.SendFirstDelayedEvent }] });
175+
this.wakeup?.({ replace: [{ ts: Date.now(), type: MembershipActionType.SendFirstDelayedEvent }] });
170176
}
171177
public initiateLeave(): void {
172-
this.wakeup?.({ setActions: [{ ts: Date.now(), type: MembershipActionType.SendScheduledDelayedLeaveEvent }] });
178+
this.wakeup?.({ replace: [{ ts: Date.now(), type: MembershipActionType.SendScheduledDelayedLeaveEvent }] });
173179
}
174180

175181
public resetState(): void {

0 commit comments

Comments
 (0)
⚡