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

(core) tweak handler for aborted connections to work on modern node

Browse Source 
Summary:
It became hard to detect aborted connections in node 16.
In node 14, req.on('close', ...) did the job. Thid diff adds a
work-around, until a better way is discovered or added.
Aborting a req will typically lead to 'close' being called
on the response, without writableFinished being set.

 - https://github.com/nodejs/node/issues/38924
 - https://github.com/nodejs/node/issues/40775

Test Plan:
existing DocApiForwarder test passes; manually
checking on various node versions.

Reviewers: JakubSerafin

Reviewed By: JakubSerafin

Differential Revision: https://phab.getgrist.com/D3923
This commit is contained in:
Paul Fitzpatrick 2023-06-15 16:14:27 -04:00
parent 52469c5a7e
commit 7e50467396
3 changed files with 21 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/gen-server/lib/DocApiForwarder.ts
View File

@ -10,6 +10,7 @@ import { IDocWorkerMap } from "app/server/lib/DocWorkerMap";
import { expressWrap } from "app/server/lib/expressWrap";
import { GristServer } from "app/server/lib/GristServer";
import { getAssignmentId } from "app/server/lib/idUtils";
import { addAbortHandler } from "app/server/lib/requestUtils";
/**
* Forwards all /api/docs/:docId/tables requests to the doc worker handling the :docId document. Makes
@ -101,7 +102,7 @@ export class DocApiForwarder {
// If the original request is aborted, abort the forwarded request too. (Currently this only
// affects some export/download requests which can abort long-running work.)
req.on('close', () => controller.abort());
addAbortHandler(req, res, () => controller.abort());
const options: RequestInit = {
method: req.method,

3
app/server/lib/ExportXLSX.ts
View File

@ -14,6 +14,7 @@
import {ActiveDoc} from 'app/server/lib/ActiveDoc';
import {ActiveDocSource, ActiveDocSourceDirect, DownloadOptions, ExportParameters} from 'app/server/lib/Export';
import log from 'app/server/lib/log';
import {addAbortHandler} from 'app/server/lib/requestUtils';
import * as express from 'express';
import contentDisposition from 'content-disposition';
import {Rpc} from 'grain-rpc';
@ -78,7 +79,7 @@ export async function streamXLSX(activeDoc: ActiveDoc, req: express.Request,
req.off('close', cancelWorker);
});
req.on('close', cancelWorker);
addAbortHandler(req, outputStream, cancelWorker);
const run = (method: string, ...args: any[]) => exportPool.run({port: port2, testDates, args}, {
name: method,

17
app/server/lib/requestUtils.ts
View File

@ -9,6 +9,7 @@ import log from 'app/server/lib/log';
import {Permit} from 'app/server/lib/Permit';
import {Request, Response} from 'express';
import _ from 'lodash';
import {Writable} from 'stream';
// log api details outside of dev environment (when GRIST_HOSTED_VERSION is set)
const shouldLogApiDetails = Boolean(process.env.GRIST_HOSTED_VERSION);
@ -335,3 +336,19 @@ export function clearSessionCacheIfNeeded(req: Request, options?: {
}) {
(req as RequestWithGrist).gristServer?.getSessions().clearCacheIfNeeded(options);
}
export function addAbortHandler(req: Request, res: Writable, op: () => void) {
// It became hard to detect aborted connections in node 16.
// In node 14, req.on('close', ...) did the job.
// The following is a work-around, until a better way is discovered
// or added. Aborting a req will typically lead to 'close' being called
// on the response, without writableFinished being set.
// https://github.com/nodejs/node/issues/38924
// https://github.com/nodejs/node/issues/40775
res.on('close', () => {
const aborted = !res.writableFinished;
if (aborted) {
op();
}
});
}