Skip to content

Commit 116c680

Browse files
alexander-akaitalexander-akait
authored
fix: do not call the next middleware when request is finished or errored (#2156)
1 parent c26a326 commit 116c680

File tree

5 files changed

+253
-84
lines changed

5 files changed

+253
-84
lines changed

‎src/index.js‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,8 @@ function koaWrapper(compiler, options) {
424424
ctx.status = statusCode;
425425
};
426426

427-
res.getReadyReadableStreamState = () => "open";
427+
let isFinished = false;
428+
let needNext = false;
428429

429430
try {
430431
await new Promise(
@@ -438,12 +439,18 @@ function koaWrapper(compiler, options) {
438439
*/
439440
res.stream = (stream) => {
440441
ctx.body = stream;
442+
443+
isFinished = true;
444+
resolve();
441445
};
442446
/**
443447
* @param {string | Buffer} data data
444448
*/
445449
res.send = (data) => {
446450
ctx.body = data;
451+
452+
isFinished = true;
453+
resolve();
447454
};
448455

449456
/**
@@ -452,6 +459,9 @@ function koaWrapper(compiler, options) {
452459
res.finish = (data) => {
453460
ctx.status = status;
454461
res.end(data);
462+
463+
isFinished = true;
464+
resolve();
455465
};
456466

457467
devMiddleware(req, res, (err) => {
@@ -460,7 +470,11 @@ function koaWrapper(compiler, options) {
460470
return;
461471
}
462472

463-
resolve();
473+
needNext = true;
474+
475+
if (!isFinished) {
476+
resolve();
477+
}
464478
});
465479
},
466480
);
@@ -475,7 +489,9 @@ function koaWrapper(compiler, options) {
475489
};
476490
}
477491

478-
await next();
492+
if (needNext) {
493+
await next();
494+
}
479495
}
480496

481497
webpackDevMiddleware.devMiddleware = devMiddleware;
@@ -575,11 +591,10 @@ function honoWrapper(compiler, options) {
575591
// Do nothing, because we set it before
576592
};
577593

578-
res.getReadyReadableStreamState = () => "readable";
579-
580594
res.getHeadersSent = () => context.env.outgoing.headersSent;
581595

582596
let body;
597+
let isFinished = false;
583598

584599
try {
585600
await new Promise(
@@ -593,7 +608,9 @@ function honoWrapper(compiler, options) {
593608
*/
594609
res.stream = (stream) => {
595610
body = stream;
596-
// responseHandler(stream);
611+
612+
isFinished = true;
613+
resolve();
597614
};
598615

599616
/**
@@ -604,9 +621,10 @@ function honoWrapper(compiler, options) {
604621
context.res.headers.delete("Content-Length");
605622

606623
body = data;
607-
};
608624

609-
let isFinished = false;
625+
isFinished = true;
626+
resolve();
627+
};
610628

611629
/**
612630
* @param {(string | Buffer)=} data data
@@ -620,8 +638,8 @@ function honoWrapper(compiler, options) {
620638
}
621639

622640
body = isDataExist ? data : null;
623-
isFinished = true;
624641

642+
isFinished = true;
625643
resolve();
626644
};
627645

‎src/middleware.js‎

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ const {
99
finish,
1010
getHeadersSent,
1111
getOutgoing,
12-
getReadyReadableStreamState,
1312
getRequestHeader,
1413
getRequestMethod,
1514
getRequestURL,
@@ -135,30 +134,33 @@ const MAX_MAX_AGE = 31536000000;
135134
*/
136135
function wrapper(context) {
137136
return async function middleware(req, res, next) {
138-
const acceptedMethods = context.options.methods || ["GET", "HEAD"];
139-
140-
initState(res);
141-
142137
/**
138+
* @param {NodeJS.ErrnoException=} err an error
143139
* @returns {Promise<void>}
144140
*/
145-
async function goNext() {
141+
async function goNext(err) {
146142
if (!context.options.serverSideRender) {
147-
return next();
143+
return next(err);
148144
}
149145

150146
return new Promise((resolve) => {
151147
ready(
152148
context,
153149
() => {
154150
setState(res, "webpack", { devMiddleware: context });
155-
resolve(next());
151+
resolve(next(err));
156152
},
157153
req,
158154
);
159155
});
160156
}
161157

158+
const acceptedMethods = context.options.methods || ["GET", "HEAD"];
159+
// TODO do we need an option here?
160+
const forwardError = false;
161+
162+
initState(res);
163+
162164
const method = getRequestMethod(req);
163165

164166
if (method && !acceptedMethods.includes(method)) {
@@ -167,11 +169,21 @@ function wrapper(context) {
167169
}
168170

169171
/**
172+
* @param {NodeJS.ErrnoException} err ann error
170173
* @param {number} status status
171174
* @param {Partial<SendErrorOptions<Request, Response>>=} options options
172-
* @returns {void}
175+
* @returns {Promise<void>}
173176
*/
174-
function sendError(status, options) {
177+
async function sendError(err, status, options) {
178+
if (forwardError) {
179+
const error =
180+
/** @type {Error & { statusCode: number }} */
181+
(new Error(err.message));
182+
error.statusCode = status;
183+
184+
await goNext(error);
185+
}
186+
175187
const escapeHtml = require("./utils/escapeHtml");
176188

177189
const content = statuses[status] || String(status);
@@ -230,19 +242,19 @@ function wrapper(context) {
230242

231243
/**
232244
* @param {NodeJS.ErrnoException} error error
233-
* @returns {void}
245+
* @returns {Promise<void>}
234246
*/
235-
function errorHandler(error) {
247+
async function errorHandler(error) {
236248
switch (error.code) {
237249
case "ENAMETOOLONG":
238250
case "ENOENT":
239251
case "ENOTDIR":
240-
sendError(404, {
252+
await sendError(error, 404, {
241253
modifyResponseData: context.options.modifyResponseData,
242254
});
243255
break;
244256
default:
245-
sendError(500, {
257+
await sendError(error, 500, {
246258
modifyResponseData: context.options.modifyResponseData,
247259
});
248260
break;
@@ -496,10 +508,15 @@ function wrapper(context) {
496508
context.logger.error(`Malicious path "${filename}".`);
497509
}
498510

499-
sendError(extra.errorCode, {
500-
modifyResponseData: context.options.modifyResponseData,
501-
});
502-
await goNext();
511+
await sendError(
512+
extra.errorCode === 400
513+
? new Error("Bad Request")
514+
: new Error("Forbidden"),
515+
extra.errorCode,
516+
{
517+
modifyResponseData: context.options.modifyResponseData,
518+
},
519+
);
503520
return;
504521
}
505522

@@ -649,8 +666,7 @@ function wrapper(context) {
649666
value = result.bufferOrStream;
650667
({ bufferOrStream, byteLength } = result);
651668
} catch (error) {
652-
errorHandler(/** @type {NodeJS.ErrnoException} */ (error));
653-
await goNext();
669+
await errorHandler(/** @type {NodeJS.ErrnoException} */ (error));
654670
return;
655671
}
656672
}
@@ -691,10 +707,9 @@ function wrapper(context) {
691707
// Conditional GET support
692708
if (isConditionalGET()) {
693709
if (isPreconditionFailure()) {
694-
sendError(412, {
710+
await sendError(new Error("Precondition Failed"), 412, {
695711
modifyResponseData: context.options.modifyResponseData,
696712
});
697-
await goNext();
698713
return;
699714
}
700715

@@ -744,13 +759,12 @@ function wrapper(context) {
744759
getValueContentRangeHeader("bytes", size),
745760
);
746761

747-
sendError(416, {
762+
await sendError(new Error("Range Not Satisfiable"), 416, {
748763
headers: {
749764
"Content-Range": getResponseHeader(res, "Content-Range"),
750765
},
751766
modifyResponseData: context.options.modifyResponseData,
752767
});
753-
await goNext();
754768
return;
755769
} else if (parsedRanges === -2) {
756770
context.logger.error(
@@ -793,8 +807,7 @@ function wrapper(context) {
793807
end,
794808
));
795809
} catch (error) {
796-
errorHandler(/** @type {NodeJS.ErrnoException} */ (error));
797-
await goNext();
810+
await errorHandler(/** @type {NodeJS.ErrnoException} */ (error));
798811
return;
799812
}
800813
}
@@ -823,7 +836,6 @@ function wrapper(context) {
823836
}
824837

825838
finish(res);
826-
await goNext();
827839
return;
828840
}
829841

@@ -838,7 +850,6 @@ function wrapper(context) {
838850

839851
if (!isPipeSupports) {
840852
send(res, /** @type {Buffer} */ (bufferOrStream));
841-
await goNext();
842853
return;
843854
}
844855

@@ -852,16 +863,11 @@ function wrapper(context) {
852863

853864
// Error handling
854865
/** @type {import("fs").ReadStream} */
855-
(bufferOrStream)
856-
.on("error", (error) => {
857-
// clean up stream early
858-
cleanup();
859-
errorHandler(error);
860-
goNext();
861-
})
862-
.on(getReadyReadableStreamState(res), () => {
863-
goNext();
864-
});
866+
(bufferOrStream).on("error", (error) => {
867+
// clean up stream early
868+
cleanup();
869+
errorHandler(error);
870+
});
865871

866872
pipe(res, /** @type {ReadStream} */ (bufferOrStream));
867873

‎src/utils/compatibleAPI.js‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
* @property {((data: any) => void)=} stream stream
2525
* @property {(() => any)=} getOutgoing get outgoing
2626
* @property {((name: string, value: any) => void)=} setState set state
27-
* @property {(() => "ready" | "open" | "readable")=} getReadyReadableStreamState get ready readable streamState
2827
*/
2928

3029
/**
@@ -301,26 +300,11 @@ function setState(res, name, value) {
301300
(res.locals)[name] = value;
302301
}
303302

304-
/**
305-
* @template {ServerResponse & ExpectedServerResponse} Response
306-
* @param {Response} res res
307-
* @returns {"ready" | "open" | "readable"} state
308-
*/
309-
function getReadyReadableStreamState(res) {
310-
// Pseudo API and Express API and Koa API
311-
if (typeof res.getReadyReadableStreamState === "function") {
312-
return res.getReadyReadableStreamState();
313-
}
314-
315-
return "ready";
316-
}
317-
318303
module.exports = {
319304
createReadStreamOrReadFileSync,
320305
finish,
321306
getHeadersSent,
322307
getOutgoing,
323-
getReadyReadableStreamState,
324308
getRequestHeader,
325309
getRequestMethod,
326310
getRequestURL,

0 commit comments

Comments
 (0)