Skip to content

Commit 6bc5072

Browse files
authored
Remove isSessionActive check for broadcasting signals (#25246)
## Description This PR aims to fix the broadcast-signal endpoint in alfred, where all requests were responding with a 410. The solution basically removes the `isSessionActive` check, as it doesn't pose a risk to the logic in general and `redis-emitter` will still be able to handle the emission without errors if the session is active. ## Reviewer Guidance This is a simple PR, removing one check that was done initially before broadcasting a signal. The removal of this check does not affect functionality or break the API. The check was removed as tests from PP resulted in the API always failing the check.
1 parent 985ae6b commit 6bc5072

File tree

2 files changed

+3
-47
lines changed
  • server/routerlicious/packages/routerlicious-base/src

2 files changed

+3
-47
lines changed

server/routerlicious/packages/routerlicious-base/src/alfred/routes/api/api.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -429,15 +429,12 @@ async function handleBroadcastSignal(
429429
}
430430

431431
const serverUrl: string = config.get("worker:serverUrl");
432-
const document = await storage?.getDocument(tenantId, documentId);
433-
if (!document?.session?.isSessionAlive) {
432+
const document = await storage.getDocument(tenantId, documentId);
433+
434+
if (!document?.session?.isSessionAlive || document?.scheduledDeletionTime) {
434435
Lumberjack.error("Document not found", { tenantId, documentId });
435436
throw new NetworkError(404, "Document not found");
436437
}
437-
if (!document.session.isSessionActive) {
438-
Lumberjack.warning("Document session not active", { tenantId, documentId });
439-
throw new NetworkError(410, "Document session not active");
440-
}
441438
if (document.session.ordererUrl !== serverUrl) {
442439
Lumberjack.info("Redirecting broadcast-signal to correct cluster", {
443440
documentUrl: document.session.ordererUrl,

server/routerlicious/packages/routerlicious-base/src/test/alfred/api.spec.ts

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,47 +1244,6 @@ describe("Routerlicious", () => {
12441244
.set("Content-Type", "application/json")
12451245
.expect(404);
12461246
});
1247-
1248-
it("Document session not active", async () => {
1249-
const body = {
1250-
signalContent: {
1251-
contents: {
1252-
type: "ExternalDataChanged_V1.0.0",
1253-
content: { taskListId: "task-list-1" },
1254-
},
1255-
},
1256-
};
1257-
const documentNoActiveSession = {
1258-
_id: "doc-1",
1259-
tenantId: appTenant1.id,
1260-
version: "1.0",
1261-
documentId: "doc-1",
1262-
content: "Hello, World!",
1263-
session: {
1264-
ordererUrl: defaultProvider.get("worker:serverUrl"),
1265-
deltaStreamUrl: defaultProvider.get("worker:deltaStreamUrl"),
1266-
historianUrl: defaultProvider.get("worker:blobStorageUrl"),
1267-
isSessionAlive: true,
1268-
isSessionActive: false,
1269-
},
1270-
createTime: Date.now(),
1271-
scribe: "",
1272-
deli: "",
1273-
};
1274-
1275-
Sinon.stub(defaultStorage, "getDocument").returns(
1276-
Promise.resolve(documentNoActiveSession),
1277-
);
1278-
1279-
await supertest
1280-
.post(
1281-
`/api/v1/${appTenant1.id}/${documentNoActiveSession._id}/broadcast-signal`,
1282-
)
1283-
.send(body)
1284-
.set("Authorization", tenantToken1)
1285-
.set("Content-Type", "application/json")
1286-
.expect(410);
1287-
});
12881247
});
12891248

12901249
describe("/documents", () => {

0 commit comments

Comments
 (0)