Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): handle the cases that res cannot be writable & add an env variable to control log #6430

Merged
merged 4 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/sweet-feet-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@modern-js/prod-server': patch
'@modern-js/server-core': patch
---

fix(server): handle the cases that res cannot be writable
fix(server): 处理 res 无法写入的情况
2 changes: 2 additions & 0 deletions packages/server/core/src/adapters/node/helper/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ export { loadServerEnv } from './loadEnv';
export { loadServerPlugins } from './loadPlugin';
export { loadServerRuntimeConfig, loadServerCliConfig } from './loadConfig';
export { loadCacheConfig } from './loadCache';
export { isResFinalized } from './utils';
export type { NodeBindings } from './utils';
33 changes: 33 additions & 0 deletions packages/server/core/src/adapters/node/helper/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import type {
HonoRequest,
NodeRequest,
NodeResponse,
ServerManifest,
} from '../../../types';

type ExtendedNodeRequest = NodeRequest & {
__honoRequest?: HonoRequest;
__templates?: Record<string, string>;
__serverManifest?: ServerManifest;
};

type ExtendedNodeResponse = NodeResponse & {
_modernBodyPiped?: boolean;
};

export type NodeBindings = {
node: {
req: ExtendedNodeRequest;
res: ExtendedNodeResponse;
};
};

export const isResFinalized = (res: ExtendedNodeResponse): boolean => {
return (
res.headersSent ||
res._modernBodyPiped ||
res.writableEnded ||
res.finished ||
!res.socket?.writable
);
};
16 changes: 2 additions & 14 deletions packages/server/core/src/adapters/node/hono.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,7 @@ import type {
ServerEnv,
ServerManifest,
} from '../../types';

type NodeBindings = {
node: {
req: NodeRequest & {
__honoRequest?: HonoRequest;
__templates?: Record<string, string>;
__serverManifest?: ServerManifest;
};
res: NodeResponse & {
_modernBodyPiped?: boolean;
};
};
};
import { type NodeBindings, isResFinalized } from './helper';

export type ServerNodeEnv = {
Bindings: NodeBindings;
Expand Down Expand Up @@ -55,7 +43,7 @@ export const httpCallBack2HonoMid = (handler: Handler) => {
res.removeListener('pipe', onPipe);
}

if (res.headersSent || res._modernBodyPiped) {
if (isResFinalized(res)) {
context.finalized = true;
} else {
await next();
Expand Down
7 changes: 2 additions & 5 deletions packages/server/core/src/adapters/node/node.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { type Server as NodeServer, ServerResponse } from 'node:http';
import type { Server as NodeHttpsServer } from 'node:https';
import type { NodeRequest, NodeResponse, RequestHandler } from '../../types';
import { isResFinalized } from './helper';
import { installGlobals } from './polyfills/install';
import {
createReadableStreamFromReadable,
Expand Down Expand Up @@ -129,11 +130,7 @@ const getRequestListener = (handler: RequestHandler) => {
* ```
*/

if (
!res.headersSent &&
!(response as any).res &&
!(res as any)._modernBodyPiped
) {
if (!(response as any).res && !isResFinalized(res)) {
await sendResponse(response, res);
}
} catch (error) {
Expand Down
4 changes: 3 additions & 1 deletion packages/server/prod-server/src/apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import type { ProdServerOptions } from './types';

function getLogger() {
if (process.env.DEBUG || process.env.NODE_ENV === 'production') {
return createLogger({ level: 'verbose' });
return createLogger({
level: (process.env.MODERN_SERVER_LOG_LEVEL as any) || 'verbose',
});
} else {
return createLogger();
}
Expand Down
2 changes: 1 addition & 1 deletion tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"main": "index.js",
"scripts": {
"test:rspack": "pnpm test:builder:rspack && pnpm test:framework && pnpm test:garfish:rspack",
"test:framework": "jest",
"test:framework": "MODERN_SERVER_LOG_LEVEL=info jest",
"test:builder:rspack": "cd e2e/builder && pnpm test:rspack",
"test:garfish:rspack": "cd e2e/garfish && pnpm test:rspack",
"test:module-tools": "cd integration && jest --testMatch **/module/**/*.test.ts",
Expand Down
Loading