Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 4 additions & 1 deletion packages/egg/src/lib/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,12 @@ export class Application extends EggApplicationCore {
super.dumpConfig();

// dump routers to router.json
const rundir = this.config.rundir;
const rundir = this.getRuntimeRundir();
Comment thread
killagu marked this conversation as resolved.
const FULLPATH = this.loader.FileLoader.FULLPATH;
try {
if (!fs.existsSync(rundir)) {
fs.mkdirSync(rundir, { recursive: true });
}
const dumpRouterFile = path.join(rundir, 'router.json');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This directory creation logic is redundant because super.dumpConfig() (called at line 200) already ensures that rundir exists using the same logic. Additionally, fs.mkdirSync with { recursive: true } is idempotent, making the fs.existsSync check unnecessary.

      const dumpRouterFile = path.join(rundir, 'router.json');

const routers = [];
for (const layer of this.router.stack) {
Expand Down
32 changes: 27 additions & 5 deletions packages/egg/src/lib/egg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import { convertObject, createTransparentProxy } from './core/utils.ts';
import type { EggApplicationLoader } from './loader/index.ts';
import type { EggAppConfig } from './types.ts';

const DEFAULT_WORKER_START_TIMEOUT = 10 * 60 * 1000;

export interface EggApplicationCoreOptions extends Omit<EggCoreOptions, 'baseDir'> {
mode?: 'cluster' | 'single';
clusterPort?: number;
Expand Down Expand Up @@ -565,10 +567,10 @@ export class EggApplicationCore extends EggCore {
* @private
*/
dumpConfig(): void {
const rundir = this.config.rundir;
const rundir = this.getRuntimeRundir();
Comment thread
killagu marked this conversation as resolved.
try {
if (!fs.existsSync(rundir)) {
fs.mkdirSync(rundir);
fs.mkdirSync(rundir, { recursive: true });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The fs.existsSync check is redundant when using fs.mkdirSync with { recursive: true }, as it is idempotent and will not throw if the directory already exists. Simplifying this improves code clarity.

      fs.mkdirSync(rundir, { recursive: true });


// get dumped object
Expand All @@ -589,7 +591,10 @@ export class EggApplicationCore extends EggCore {
dumpTiming(): void {
try {
const items = this.timing.toJSON();
const rundir = this.config.rundir;
const rundir = this.getRuntimeRundir();
if (!fs.existsSync(rundir)) {
fs.mkdirSync(rundir, { recursive: true });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The fs.existsSync check is redundant here as well since fs.mkdirSync with { recursive: true } handles existence checks internally.

      fs.mkdirSync(rundir, { recursive: true });

const dumpFile = path.join(rundir, `${this.type}_timing_${process.pid}.json`);
fs.writeFileSync(dumpFile, CircularJSON.stringify(items, null, 2));
this.coreLogger.info(this.timing.toString());
Expand Down Expand Up @@ -641,9 +646,10 @@ export class EggApplicationCore extends EggCore {
}

#setupTimeoutTimer(): void {
const workerStartTimeout = this.getWorkerStartTimeout();
const startTimeoutTimer = setTimeout(() => {
this.coreLogger.error(this.timing.toString());
this.coreLogger.error(`${this.type} still doesn't ready after ${this.config.workerStartTimeout} ms.`);
this.coreLogger.error(`${this.type} still doesn't ready after ${workerStartTimeout} ms.`);
Comment thread
killagu marked this conversation as resolved.
// log unfinished
const items = this.timing.toJSON();
for (const item of items) {
Expand All @@ -658,10 +664,26 @@ export class EggApplicationCore extends EggCore {
this.emit('startTimeout');
this.dumpConfig();
this.dumpTiming();
}, this.config.workerStartTimeout);
}, workerStartTimeout);
this.ready(() => clearTimeout(startTimeoutTimer));
}

protected getRuntimeRundir(): string {
const rundir = this.config.rundir;
if (typeof rundir === 'string' && rundir.length > 0) {
return rundir;
}
return path.join(this.baseDir, 'run');
}
Comment thread
killagu marked this conversation as resolved.

private getWorkerStartTimeout(): number {
const workerStartTimeout = this.config.workerStartTimeout;
if (typeof workerStartTimeout === 'number' && Number.isFinite(workerStartTimeout)) {
return workerStartTimeout;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return DEFAULT_WORKER_START_TIMEOUT;
}

get config() {
return super.config as EggAppConfig;
}
Expand Down
39 changes: 39 additions & 0 deletions packages/egg/test/egg.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,45 @@ describe.sequential('test/egg.test.ts', () => {
});
});

describe('runtime diagnostics fallback config', () => {
const baseDir = getFilepath('apps/dumpconfig');
const runDir = path.join(baseDir, 'run');
let app: MockApplication;

beforeAll(async () => {
app = createApp('apps/dumpconfig');
await app.ready();
});

afterAll(() => app.close());

it('should dump config and timing to baseDir/run when rundir is missing', () => {
fs.rmSync(runDir, { recursive: true, force: true });
const originalRundir = app.config.rundir;
Reflect.set(app.config, 'rundir', undefined);
try {
app.dumpConfig();
app.dumpTiming();
} finally {
Reflect.set(app.config, 'rundir', originalRundir);
}

assertFile(path.join(runDir, 'application_config.json'));
assertFile(path.join(runDir, `application_timing_${process.pid}.json`));
assertFile(path.join(runDir, 'router.json'));
});

it('should use the default worker start timeout when config is missing', () => {
const originalWorkerStartTimeout = app.config.workerStartTimeout;
Reflect.set(app.config, 'workerStartTimeout', undefined);
try {
assert.equal((app as any).getWorkerStartTimeout(), 10 * 60 * 1000);
} finally {
Reflect.set(app.config, 'workerStartTimeout', originalWorkerStartTimeout);
}
});
});

describe('custom config from env', () => {
let app: MockApplication;
let baseDir: string;
Expand Down
Loading