Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

Fix deadlock with webhooks on document load #799 (#812)

Browse Source
This commit is contained in:
Florent 2024-01-03 20:47:53 +01:00 committed by GitHub
parent 6722512d96
commit 837597cd55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
app/server/lib/Sharing.ts
View File

@ -22,6 +22,7 @@ import {ActionHistory, asActionGroup, getActionUndoInfo} from './ActionHistory';
import {ActiveDoc} from './ActiveDoc';
import {makeExceptionalDocSession, OptDocSession} from './DocSession';
import {WorkCoordinator} from './WorkCoordinator';
import {summarizeAction} from 'app/common/ActionSummarizer';
// Describes the request to apply a UserActionBundle. It includes a Client (so that broadcast
// message can set `.fromSelf` property), and methods to resolve or reject the promise for when
@ -246,14 +247,14 @@ export class Sharing {
try {
const isCalculate = (userActions.length === 1 && SYSTEM_ACTIONS.has(userActions[0][0] as string));
const isSystemAction = (userActions.length === 1 && SYSTEM_ACTIONS.has(userActions[0][0] as string));
// `internal` is true if users shouldn't be able to undo the actions. Applies to:
// - Calculate/UpdateCurrentTime because it's not considered as performed by a particular client.
// - Adding attachment metadata when uploading attachments,
// because then the attachment file may get hard-deleted and redo won't work properly.
// - Action was rejected but it had some side effects (e.g. NOW() or UUID() formulas).
const internal =
isCalculate ||
isSystemAction ||
userActions.every(a => a[0] === "AddRecord" && a[1] === "_grist_Attachments") ||
!!failure;
@ -305,7 +306,7 @@ export class Sharing {
// If the document has shut down in the meantime, and this was just a "Calculate" action,
// return a trivial result. This is just to reduce noisy warnings in migration tests.
if (this._activeDoc.isShuttingDown && isCalculate) {
if (this._activeDoc.isShuttingDown && isSystemAction) {
return {
actionNum: localActionBundle.actionNum,
retValues: [],
@ -336,7 +337,12 @@ export class Sharing {
}
await this._activeDoc.processActionBundle(ownActionBundle);
const actionSummary = await this._activeDoc.handleTriggers(localActionBundle);
// Don't trigger webhooks for single Calculate actions, this causes a deadlock on document load.
// See gh issue #799
const isSingleCalculateAction = userActions.length === 1 && userActions[0][0] === 'Calculate';
const actionSummary = !isSingleCalculateAction ?
await this._activeDoc.handleTriggers(localActionBundle) :
summarizeAction(localActionBundle);
await this._activeDoc.updateRowCount(sandboxActionBundle.rowCount, docSession);

31
test/server/lib/DocApi.ts
View File

@ -4599,6 +4599,37 @@ function testDocApi() {
await unsubscribe1();
});
it('should not block document load (gh issue #799)', async function () {
// Create a test document.
const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id;
const docId = await userApi.newDoc({name: 'testdoc5'}, ws1);
const doc = userApi.getDocAPI(docId);
// Before #799, formula of this type would block document load because of a deadlock
// and make this test fail.
const formulaEvaluatedAtDocLoad = 'NOW()';
await axios.post(`${serverUrl}/api/docs/${docId}/apply`, [
['ModifyColumn', 'Table1', 'C', {isFormula: true, formula: formulaEvaluatedAtDocLoad}],
], chimpy);
const unsubscribeWebhook1 = await autoSubscribe('probe', docId);
// Create a first row.
await doc.addRows("Table1", {
A: [1],
});
await doc.forceReload();
// Create a second row after document reload.
// This should not timeout.
await doc.addRows("Table1", {
A: [2],
});
await unsubscribeWebhook1();
});
it("should monitor failures", async () => {
const webhook3 = await subscribe('probe', docId);
const webhook4 = await subscribe('probe', docId);