Conversation
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new @eggjs/onerror plugin (source, config, templates, types, tests, packaging, docs). Migrates mock package typings from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Koa as Koa App
participant App as Egg Application
participant OnErr as @eggjs/onerror
participant Utils as onerror utils
participant View as ErrorView/template
Client->>Koa: HTTP request
Koa->>App: execute route / middleware
App-->>Koa: throws Error (err)
Koa->>OnErr: onerror handler invoked (ctx, err)
OnErr->>Utils: detectStatus(err) / accepts(ctx)
alt accepts === html
OnErr->>OnErr: maybe redirect via config.errorPageUrl
OnErr->>View: render HTML via ErrorView
View-->>OnErr: HTML body
OnErr-->>Client: respond HTML (status)
else accepts === json
OnErr-->>Client: respond JSON body (code/message[/stack])
else accepts === js/jsonp
OnErr-->>Client: respond JSONP/JS via ctx.createJsonpBody
end
sequenceDiagram
autonumber
participant Agent as Egg Agent
participant Boot as onerror Agent Boot
participant Logger as agent.coreLogger
Agent->>Boot: new Boot(agent)
Boot->>Agent: didLoad() registers agent.on('error', handler)
Agent-->>Boot: emits 'error' (err)
Boot->>Logger: coreLogger.error(err)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying egg with
|
| Latest commit: |
4983cb2
|
| Status: | 🚫 Build failed. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request integrates the @eggjs/onerror plugin directly into the monorepo, streamlining its management and ensuring consistency with other internal packages. The migration involves updating its package.json for workspace compatibility, configuring tsdown for standardized builds, and setting up vitest for testing. Additionally, it includes minor fixes and enhancements to the error handling and display logic, ensuring a robust and well-integrated error reporting mechanism across the project.
Highlights
- Monorepo Integration: The
@eggjs/onerrorplugin has been moved from a separate repository into the main monorepo, centralizing its development and maintenance. - Standardized Build & Dependencies: The plugin now uses workspace dependencies and a standard
tsdownbuild configuration, aligning with the monorepo's conventions. - Updated Type Definitions & Testing: New type definitions for
mustacheandstack-tracehave been added, andvitestis configured for testing the plugin. - Error Page Enhancements: The error page rendering logic has been refined, including a fix for an HTML syntax error in the Mustache template and improved handling of various error statuses and content types.
- Dependency Catalog Updates: Several new dependencies, including
cookie,koa-onerror,mustache, andstack-trace, have been added to the pnpm catalog.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Pull Request Overview
This PR merges the standalone @eggjs/onerror plugin into the monorepo structure, bringing it in line with other plugins and providing standardized build tooling. The plugin handles error responses across different formats (HTML, JSON, JSONP) with production-safe error display.
- Move @eggjs/onerror from separate repository to plugins/onerror directory
- Standardize monorepo configuration with workspace dependencies and tsdown builds
- Add required dependencies to pnpm catalog and fix HTML template syntax error
Reviewed Changes
Copilot reviewed 68 out of 70 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plugins/onerror/* | Complete onerror plugin with src/, test/, and config files |
| pnpm-workspace.yaml | Add new dependencies: cookie, koa-onerror, mustache, stack-trace with types |
| tools/egg-bin/tsconfig.json | Simplify tsconfig to extend from root configuration |
| packages/*/tsconfig.json | Add baseUrl configuration for consistent path resolution |
| packages/mock/src/* | Update imports from @eggjs/core to egg for better type alignment |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
plugins/onerror/test/fixtures/onerror-custom-template/template.mustache
Outdated
Show resolved
Hide resolved
Deploying egg-v3 with
|
| Latest commit: |
4983cb2
|
| Status: | 🚫 Build failed. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
There was a problem hiding this comment.
Code Review
This pull request successfully merges the @eggjs/onerror plugin into the monorepo. The changes are well-structured, including the addition of the new plugin, configuration updates, and refactoring of related packages. The inclusion of comprehensive tests is a great addition. I have identified a few areas for improvement concerning code conciseness, correctness, and performance, along with a syntax error in an HTML template. My detailed feedback is provided in the review comments.
plugins/onerror/test/fixtures/onerror-custom-template/template.mustache
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #5503 +/- ##
==========================================
- Coverage 76.34% 76.31% -0.03%
==========================================
Files 113 113
Lines 5885 5891 +6
Branches 1233 1237 +4
==========================================
+ Hits 4493 4496 +3
- Misses 1215 1218 +3
Partials 177 177 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/mock/src/app/middleware/cluster_app_mock.ts (3)
15-16: Possible crash: args may be undefined (accessing length).
argscan be absent; laterargs.lengthwill throw. Normalize early and validate against the raw value.- const { method, property, args, needResult } = body; + const { method, property, args: rawArgs, needResult } = body ?? {}; + const args: any[] = Array.isArray(rawArgs) ? [...rawArgs] : []; @@ - if (args && !Array.isArray(args)) { + if (rawArgs !== undefined && !Array.isArray(rawArgs)) { ctx.status = 422; ctx.body = { success: false, error: 'args should be an Array instance', }; return; } @@ - for (let i = 0; i < args.length; i++) { + for (let i = 0; i < args.length; i++) { const arg = args[i];Also applies to: 24-31, 71-90
95-101: Async handling bug: response may be set after middleware completes.
then()withoutawaitrisks sending response beforectx.bodyis assigned and drops rejections. Await the call inside try/catch.- Promise.resolve(fn.call(target, ...args)).then(result => { - ctx.body = needResult ? { success: true, result } : { success: true }; - }); + const result = await Promise.resolve(fn.call(target, ...args)); + ctx.body = needResult ? { success: true, result } : { success: true }; } catch (err: any) { ctx.status = 500; ctx.body = { success: false, error: err.message }; }
75-78: RCE surface via eval in request body. Gate to unittest only.Evaluating arbitrary code from the request is dangerous if this middleware is reachable outside tests. Gate by env or remove in non‑test envs.
- // eslint-disable-next-line - args[i] = eval(`(function() { return ${arg.value} })()`); + if (ctx.app.config.env === 'unittest') { + // eslint-disable-next-line no-eval + args[i] = eval(`(function() { return ${arg.value} })()`); + } else { + ctx.status = 403; + ctx.body = { success: false, error: 'mock function eval is disabled outside unittest env' }; + return; + }packages/mock/src/lib/parallel/agent.ts (2)
105-113: Preserve once() semantics during pre-init.Using super.on inside once() can invoke the listener multiple times before binding. Use super.once.
Apply this diff:
- super.on(args[0], args[1]); + super.once(args[0], args[1]);
1-132: Fix invalidtypeofon type-only imports and pre-initoncehandler
- Type-only
typeoferror — packages/mock/src/lib/parallel/agent.ts:55 and packages/mock/src/lib/parallel/app.ts:44: replace{ Agent: typeof EggAgent }/{ Application: typeof EggApplication }with{ Agent: typeof import('egg').Agent }/{ Application: typeof import('egg').Application }(or switch to value imports instead ofimport type).- once() cache bug — packages/mock/src/lib/parallel/agent.ts (≈lines 105–112): the pre-init
once(...args)branch callssuper.on(args[0], args[1]); change that tosuper.once(args[0], args[1]).
♻️ Duplicate comments (6)
plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/user.js (1)
3-5: Same status parsing hardening as in onerror-4xx/user.js.Replicate the integer parsing/validation to keep fixtures consistent.
Also applies to: 6-8
plugins/onerror/test/fixtures/onerror-customize/app/controller/user.js (1)
3-5: Mirror the status parsing fix here for consistency.Keep all three user controllers aligned to reduce test flakiness on malformed input.
Also applies to: 6-8
plugins/onerror/test/fixtures/onerror-custom-template/template.mustache (1)
114-114: Fix malformed closing tag.The tag is still
</<div>; should be</div>.- </<div> + </div>plugins/onerror/src/agent.ts (1)
6-8: Use constructor parameter property for brevity.Same as prior feedback; keeps it concise without changing behavior.
-export default class Boot implements ILifecycleBoot { - private agent: Agent; - - constructor(agent: Agent) { - this.agent = agent; - } +export default class Boot implements ILifecycleBoot { + constructor(private readonly agent: Agent) {}plugins/onerror/src/app.ts (1)
136-139: Preserve falsy values when merging error fields.Using
if (!errorJson[key])skips valid falsy values (0, false, ''). Check key existence instead.- for (const key in err) { - if (!errorJson[key]) { - errorJson[key] = (err as any)[key]; - } - } + for (const [key, value] of Object.entries(err as Record<string, any>)) { + if (!(key in errorJson)) { + errorJson[key] = value; + } + }plugins/onerror/src/lib/error_view.ts (1)
274-276: Simplify check for dumpConfigToObject.
inis redundant withtypeofguard.- if ('dumpConfigToObject' in this.app && typeof this.app.dumpConfigToObject === 'function') { + if (typeof this.app.dumpConfigToObject === 'function') {
🧹 Nitpick comments (46)
plugins/onerror/test/fixtures/onerror-custom-500/package.json (1)
1-3: Mark fixture as private to avoid accidental publish/workspace pickup.Add
"private": trueto ensure this test fixture can’t be published or treated as a workspace package.{ - "name": "onerror-custom-500" + "name": "onerror-custom-500", + "private": true }plugins/onerror/test/fixtures/onerror-customize/package.json (1)
1-3: Set"private": truefor test fixture packages.Prevents accidental publishing and workspace linking.
{ - "name": "onerror-customize" + "name": "onerror-customize", + "private": true }plugins/onerror/test/fixtures/onerror-4xx/package.json (1)
1-3: Make fixture package private.Same rationale as other fixtures.
{ - "name": "onerror-4xx" + "name": "onerror-4xx", + "private": true }plugins/onerror/test/fixtures/onerror-no-errorpage/package.json (1)
1-3: Mark as private.Avoids accidental publish/workspace inclusion.
{ - "name": "onerror-no-errorpage" + "name": "onerror-no-errorpage", + "private": true }plugins/onerror/test/fixtures/onerror-ctx-error/package.json (1)
1-3: Mark as private.Same fixture hardening as others.
{ - "name": "onerror-ctx-error" + "name": "onerror-ctx-error", + "private": true }plugins/onerror/test/fixtures/onerror-custom-template/package.json (1)
1-3: Mark as private.Prevents accidental publication.
{ - "name": "onerror-customize-template" + "name": "onerror-customize-template", + "private": true }plugins/onerror/tsconfig.json (1)
2-7: Confirm strict mode is effectively enabled.If root tsconfig doesn’t set
"strict": true, add it here to comply with repo-wide strict TS policy.{ "extends": "../../tsconfig.json", "compilerOptions": { - "baseUrl": "./" + "baseUrl": "./", + "strict": true }, "exclude": ["test/fixtures/**/*.ts", "dist/**/*"] }packages/mock/src/app/middleware/cluster_app_mock.ts (2)
33-46: Harden property checks to avoid prototype exposure.Use own‑property checks when reading
ctx.app[property]to avoid__proto__/prototype leakage.- if (method === '__getter__') { - if (!ctx.app[property]) { + if (method === '__getter__') { + if (!Object.prototype.hasOwnProperty.call(ctx.app, property)) { debug('property %s not exists on app', property); ctx.status = 422; ctx.body = { success: false, error: `property "${property}" not exists on app`, }; return; } ctx.body = { success: true, result: ctx.app[property] }; return; } @@ - if (!ctx.app[property] || typeof (ctx.app as any)[property][method] !== 'function') { + if (!Object.prototype.hasOwnProperty.call(ctx.app, property) || + typeof (ctx.app as any)[property][method] !== 'function') { debug('property %s.%s not exists on app', property, method); ctx.status = 422; ctx.body = { success: false, error: `method "${method}" not exists on app.${property}`, }; return; }Also applies to: 48-57
10-13: Optionally gate whole endpoint to unittest env.If this middleware can be mounted in non‑test environments, short‑circuit early.
if (ctx.originalUrl !== '/__egg_mock_call_function') { return next(); } + if (ctx.app.config.env !== 'unittest') { + return next(); + }plugins/onerror/test/fixtures/onerror/package.json (1)
1-3: Mark test fixtures as private to prevent accidental publish.Apply:
{ "name": "onerror", + "private": true }plugins/onerror/test/fixtures/custom-listener-onerror/package.json (1)
1-3: Mark test fixtures as private to prevent accidental publish.{ "name": "custom-listener-onerror", + "private": true }plugins/onerror/test/fixtures/agent-error/package.json (1)
1-3: Mark test fixtures as private to prevent accidental publish.{ "name": "agent-error", + "private": true }packages/mock/src/app/extend/agent.ts (3)
13-14: Fix strictPropertyInitialization for _mockHttpClient.Under strict mode this field is never assigned in constructor. Use definite assignment.
- _mockHttpClient: MockHttpClientMethod; + _mockHttpClient!: MockHttpClientMethod;
15-19: Correct JSDoc receiver: should be Agent, not App.Keeps docs consistent with the class name.
- * @function App#mockHttpclient + * @function Agent#mockHttpclient
24-27: Correct JSDoc receiver on second method, too.- * @function App#mockHttpClient + * @function Agent#mockHttpClientplugins/onerror/test/fixtures/onerror/app/controller/user.js (1)
6-9: Optionally parse JSON for errors param in tests.Improves flexibility when passing errors as a JSON string.
- if (ctx.query.errors) { - err.errors = ctx.query.errors; - } + if (ctx.query.errors) { + const q = ctx.query.errors; + if (typeof q === 'string') { + try { err.errors = JSON.parse(q); } catch { err.errors = q; } + } else { + err.errors = q; + } + }plugins/onerror/test/fixtures/onerror-custom-template/app/controller/user.js (1)
6-9: Same optional parsing as above for consistency across fixtures.- if (ctx.query.errors) { - err.errors = ctx.query.errors; - } + if (ctx.query.errors) { + const q = ctx.query.errors; + if (typeof q === 'string') { + try { err.errors = JSON.parse(q); } catch { err.errors = q; } + } else { + err.errors = q; + } + }plugins/onerror/test/fixtures/onerror-4xx/app/controller/user.js (1)
3-5: Guard status parsing to avoid NaN/float in err.status.Use integer parsing with validation; current Number() could leave NaN or floats.
Apply:
- if (ctx.query.status) { - err.status = Number(ctx.query.status); - } + if (ctx.query.status != null) { + const s = Number.parseInt(String(ctx.query.status), 10); + if (Number.isInteger(s)) err.status = s; + }Also applies to: 6-8
plugins/onerror/test/fixtures/onerror-ctx-error/app/extend/context.js (1)
3-3: Nit: fix backtick in message (“can`t”) to an apostrophe or avoid contraction.If tests assert the exact message, prefer “cannot” to dodge escaping.
- throw new Error('you can`t get userId.'); + throw new Error('you cannot get userId.');plugins/onerror/test/fixtures/onerror-ctx-error/app/middleware/trigger.js (1)
4-6: Nit: typo and missing semicolon.Purely cosmetic; fix “happend” → “happened” and add a semicolon.
- ctx.logger.info('log something, then error happend.'); - ctx.logger.info('%s', ctx.userId) + ctx.logger.info('log something, then error happened.'); + ctx.logger.info('%s', ctx.userId);plugins/onerror/test/fixtures/custom-listener-onerror/config/config.default.js (1)
5-7: Prefer ctx.logger for request-scoped logging.Use ctx.logger to keep request context in logs.
- ctx.app.logger.error('error happened'); + ctx.logger.error('error happened');plugins/onerror/CHANGELOG.md (1)
1-4: Silence markdownlint for imported historical changelog.Keep upstream formatting; disable specific rules locally.
+<!-- markdownlint-disable MD003 MD034 MD036 MD007 --> # Changelogplugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js (1)
6-8: Parse status robustly (avoid falsy '0' and NaN).Guard Number() results; same for both handlers.
- if (ctx.query.status) { - err.status = Number(ctx.query.status); - } + { + const status = Number(ctx.query.status); + if (!Number.isNaN(status)) err.status = status; + }Apply in both places.
Also applies to: 22-24
plugins/onerror/test/fixtures/onerror-4xx/app/controller/home.js (1)
6-8: Same status parsing nit as the other fixture.Use a NaN check to avoid accidental coercion/falsy pitfalls.
- if (ctx.query.status) { - err.status = Number(ctx.query.status); - } + { + const status = Number(ctx.query.status); + if (!Number.isNaN(status)) err.status = status; + }Also applies to: 22-24
plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js (1)
6-8: Guard against NaN and invalid HTTP status when parsing from query.Number(ctx.query.status) can yield NaN; prefer a finite integer check.
- if (ctx.query.status) { - err.status = Number(ctx.query.status); - } + if (ctx.query.status != null) { + const s = Number.parseInt(String(ctx.query.status), 10); + if (Number.isFinite(s)) err.status = s; + }plugins/onerror/src/lib/utils.ts (2)
6-9: Pass content types to ctx.request.is as an array to avoid TS/overload ambiguity.This also keeps parity with type-is’ array form.
- ctx.request.is('application/json', 'application/vnd.api+json', 'application/csp-report')) { + ctx.request.is([ + 'application/json', + 'application/vnd.api+json', + 'application/csp-report', + ])) {
24-28: Explicitly type the return union of accepts().Minor clarity win for downstream call sites.
-export function accepts(ctx: Context) { +export function accepts(ctx: Context): 'json' | 'js' | 'html' {packages/mock/src/app/extend/application.ts (1)
249-251: Use boolean for “secure” on the mocked request.Current value is a string; make it a boolean to match expectations.
- secure: 'false', + secure: false,plugins/onerror/test/fixtures/onerror/app/controller/home.js (1)
16-18: Defensive: err.stack may be undefined.Avoid TypeError when Error lacks a stack in some runtimes.
- err.stack = err.stack.replace(/(controller\/home\.)js/, '$1ts'); + if (err.stack) { + err.stack = err.stack.replace(/(controller\/home\.)js/, '$1ts'); + }plugins/onerror/test/fixtures/onerror-custom-template/template.mustache (1)
16-16: Optional: add alt text to the decorative image.Improves a11y.
- <img class="error-logo" src="https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg"/> + <img class="error-logo" alt="" src="https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg"/>plugins/onerror/src/lib/onerror_page.mustache.html (2)
5-5: Optional: populate the document title.Small UX improvement for browser tabs.
- <title></title> + <title>{{ status }} {{ name }} — {{ request.url }}</title>
581-581: Optional: add alt text to the logo image.- <img class="error-logo" src="https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg"/> + <img class="error-logo" alt="" src="https://zos.alipayobjects.com/rmsportal/JFKAMfmPehWfhBPdCjrw.svg"/>plugins/onerror/package.json (2)
4-16: Expose .d.ts for all subpath exports via publishConfig.exports.TypeScript won’t pick up declarations for subpaths (e.g., ./lib/utils) unless you add a "types" mapping per export (or typesVersions). Recommend switching string targets to objects with "types" + "default".
Apply this diff:
"publishConfig": { "access": "public", - "exports": { - ".": "./dist/index.js", - "./agent": "./dist/agent.js", - "./app": "./dist/app.js", - "./config/config.default": "./dist/config/config.default.js", - "./lib/error_view": "./dist/lib/error_view.js", - "./lib/utils": "./dist/lib/utils.js", - "./types": "./dist/types.js", - "./package.json": "./package.json" - } + "exports": { + ".": { "types": "./dist/index.d.ts", "default": "./dist/index.js" }, + "./agent": { "types": "./dist/agent.d.ts", "default": "./dist/agent.js" }, + "./app": { "types": "./dist/app.d.ts", "default": "./dist/app.js" }, + "./config/config.default": { + "types": "./dist/config/config.default.d.ts", + "default": "./dist/config/config.default.js" + }, + "./lib/error_view": { + "types": "./dist/lib/error_view.d.ts", + "default": "./dist/lib/error_view.js" + }, + "./lib/utils": { + "types": "./dist/lib/utils.d.ts", + "default": "./dist/lib/utils.js" + }, + "./types": { "types": "./dist/types.d.ts", "default": "./dist/types.js" }, + "./package.json": "./package.json" + } },
24-28: Homepage branch pin.Linking to tree/master can drift; prefer HEAD to follow the default branch automatically.
- "homepage": "https://github.com/eggjs/egg/tree/master/plugins/onerror#readme", + "homepage": "https://github.com/eggjs/egg/tree/HEAD/plugins/onerror#readme",plugins/onerror/test/fixtures/agent-error/agent.js (1)
1-6: Speed up/flakiness: avoid 500ms sleep in tests.Use setImmediate or a tiny timeout to reduce test time and flakes.
- setTimeout(() => { + setImmediate(() => { done(new Error('emit error')); - }, 500); + });plugins/onerror/src/types.ts (1)
1-6: Tighten re-export.Inline the type re-export and drop the separate import.
-import type { - OnerrorConfig, -} from './config/config.default.ts'; - -export type { OnerrorConfig }; +export type { OnerrorConfig } from './config/config.default.ts';plugins/onerror/src/app.ts (1)
26-27: Guard template loading for resilience.If the template path is missing or unreadable,
readFileSyncwill fail at boot. Consider try/catch with a minimal fallback template to avoid crashing startup.- const viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); + let viewTemplate = ''; + try { + viewTemplate = fs.readFileSync(config.templatePath, 'utf8'); + } catch (e) { + this.app.logger.warn('[onerror] failed to read template: %s', e); + viewTemplate = '<h1>Internal Server Error</h1>'; + }plugins/onerror/test/onerror.test.ts (3)
115-119: Fix typo in test title.Minor spelling nit: “accpets” → “accepts”.
- it('should support custom accpets return err.stack', () => { + it('should support custom accepts return err.stack', () => {
324-326: Remove duplicate assignment.
app = app = mm.app(...)assigns twice.- app = app = mm.app({ + app = mm.app({
344-346: Remove duplicate assignment (second occurrence).- app = app = mm.app({ + app = mm.app({plugins/onerror/src/lib/error_view.ts (5)
15-15: Anchor the “starting slash” regex.Current
/\\|\\//removes the first slash anywhere, not just at the start. Anchor to start to address CodeQL warning and intent.-const startingSlashRegex = /\\|\//; +const startingSlashRegex = /^(?:[\\/]+)/;
201-205: Prefer removing only leading CWD and normalize consistently.With the anchored regex above, this will reliably trim leading separators after stripping
process.cwd().
127-142: Guard against missing filenames and avoid sync I/O on hot path.
- If
frame.getFileName()returns falsy,existsSync/readFileSynccan throw.- Synchronous I/O can block even in local env with large stacks.
- getFrameSource(frame: StackFrame): FrameSource { - const filename = frame.getFileName(); + getFrameSource(frame: StackFrame): FrameSource { + const filename = frame.getFileName(); + if (!filename) { + return { pre: [], line: '', post: [] }; + } const lineNumber = frame.getLineNumber(); let contents = this.getAssets(filename); if (!contents) { contents = fs.existsSync(filename) ? fs.readFileSync(filename, 'utf8') : ''; this.setAssets(filename, contents); }Optional follow‑up (non‑blocking): make
toHTML/parseError/getFrameSourceasync and usefs.promises.readFile, keeping a small LRU cache.
244-252: Redact additional sensitive headers.Consider expanding
_filterHeadersto avoid leaking secrets on dev error pages (authorization tokens, API keys, CSRF tokens).- _filterHeaders = [ 'cookie', 'connection' ]; + _filterHeaders = [ + 'cookie', + 'connection', + 'authorization', + 'proxy-authorization', + 'x-api-key', + 'x-csrf-token', + 'x-xsrf-token', + 'x-egg-session', + ];Also applies to: 35-37
223-236: Use Array.isArray for robustness.Minor readability/semantics improvement.
- frames: stack instanceof Array ? stack.filter(frame => frame.getFileName()).map(frameFormatter) : [], + frames: Array.isArray(stack) ? stack.filter(frame => frame.getFileName()).map(frameFormatter) : [],packages/mock/src/lib/parallel/agent.ts (1)
79-91: Clear cached listener sets after binding to avoid leaks.After migrating listeners to the agent, clear the caches.
Apply this diff:
#bindEvents() { @@ for (const args of this.#initOnceListeners) { debug('once(%s), use cache and pass to agent', args); this._instance.once(args[0], args[1]); this.removeListener(args[0], args[1]); } + this.#initOnListeners.clear(); + this.#initOnceListeners.clear(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (69)
packages/koa/tsconfig.json(1 hunks)packages/mock/package.json(1 hunks)packages/mock/src/app.ts(1 hunks)packages/mock/src/app/extend/agent.ts(2 hunks)packages/mock/src/app/extend/application.ts(2 hunks)packages/mock/src/app/middleware/cluster_app_mock.ts(1 hunks)packages/mock/src/lib/context.ts(0 hunks)packages/mock/src/lib/parallel/agent.ts(3 hunks)packages/mock/src/lib/parallel/app.ts(3 hunks)packages/supertest/tsconfig.json(1 hunks)packages/utils/tsconfig.json(1 hunks)plugins/onerror/CHANGELOG.md(1 hunks)plugins/onerror/LICENSE(1 hunks)plugins/onerror/README.md(1 hunks)plugins/onerror/package.json(1 hunks)plugins/onerror/src/agent.ts(1 hunks)plugins/onerror/src/app.ts(1 hunks)plugins/onerror/src/config/config.default.ts(1 hunks)plugins/onerror/src/index.ts(1 hunks)plugins/onerror/src/lib/error_view.ts(1 hunks)plugins/onerror/src/lib/onerror_page.mustache.html(1 hunks)plugins/onerror/src/lib/utils.ts(1 hunks)plugins/onerror/src/types.ts(1 hunks)plugins/onerror/test/fixtures/agent-error/agent.js(1 hunks)plugins/onerror/test/fixtures/agent-error/package.json(1 hunks)plugins/onerror/test/fixtures/custom-listener-onerror/app/router.js(1 hunks)plugins/onerror/test/fixtures/custom-listener-onerror/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/custom-listener-onerror/package.json(1 hunks)plugins/onerror/test/fixtures/mock-test-error/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/app/extend/context.js(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/app/middleware/trigger.js(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-custom-500/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-500/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-500/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/template.mustache(1 hunks)plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/package.json(1 hunks)plugins/onerror/test/fixtures/onerror/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror/package.json(1 hunks)plugins/onerror/test/onerror.test.ts(1 hunks)plugins/onerror/tsconfig.json(1 hunks)plugins/onerror/tsdown.config.ts(1 hunks)plugins/onerror/vitest.config.ts(1 hunks)pnpm-workspace.yaml(4 hunks)tools/egg-bin/tsconfig.json(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/mock/src/lib/context.ts
🧰 Additional context used
📓 Path-based instructions (13)
{packages,plugins,tools}/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins,tools}/**/package.json: Individual packages must reference external dependencies via "catalog:" from pnpm-workspace.yaml
Use "workspace:*" for internal monorepo dependencies between packages
Files:
plugins/onerror/test/fixtures/onerror-custom-500/package.jsonplugins/onerror/test/fixtures/mock-test-error/package.jsonplugins/onerror/test/fixtures/agent-error/package.jsonplugins/onerror/test/fixtures/onerror/package.jsonplugins/onerror/test/fixtures/onerror-customize/package.jsonplugins/onerror/test/fixtures/onerror-ctx-error/package.jsonplugins/onerror/test/fixtures/onerror-custom-template/package.jsonpackages/mock/package.jsonplugins/onerror/test/fixtures/onerror-no-errorpage/package.jsonplugins/onerror/test/fixtures/onerror-4xx/package.jsonplugins/onerror/test/fixtures/custom-listener-onerror/package.jsonplugins/onerror/package.json
packages/*/tsconfig.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All sub-project tsconfig.json files must extend from the root via "extends": "../../tsconfig.json"
Files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsonpackages/utils/tsconfig.json
{tsconfig.json,packages/*/tsconfig.json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable strict TypeScript mode across all packages
Files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsonpackages/utils/tsconfig.jsontsconfig.json
{packages,plugins,tools}/**/tsconfig.json
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins,tools}/**/tsconfig.json: All sub-project tsconfig.json files must extend the root tsconfig.json using "extends": "../../tsconfig.json"
All sub-project tsconfig.json files must include compilerOptions.baseUrl set to "./"
Files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsontools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.json
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use TypeScript for all source files in packages (no JavaScript source files)
Files:
packages/mock/src/app/extend/agent.tspackages/mock/src/app/middleware/cluster_app_mock.tspackages/mock/src/lib/parallel/app.tspackages/mock/src/app.tspackages/mock/src/lib/parallel/agent.tspackages/mock/src/app/extend/application.ts
packages/*/package.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use "workspace:*" for internal workspace dependencies in package.json
Files:
packages/mock/package.json
{packages,plugins}/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins}/**/test/**/*.test.ts: Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
Use Node.js built-in assert for assertions in Vitest tests
Files:
plugins/onerror/test/onerror.test.ts
pnpm-workspace.yaml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Define external dependency versions centrally in the catalog section of pnpm-workspace.yaml
Files:
pnpm-workspace.yaml
plugins/**/tsdown.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All plugins must use the standard tsdown configuration template (entry: 'src/**/*.ts', unbundle: true, dts: true, exports.devExports: true)
Files:
plugins/onerror/tsdown.config.ts
{packages,plugins,tools}/**/tsdown.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each package must use tsdown for unbundled ESM builds with type declarations (unbundle: true, dts: true)
Files:
plugins/onerror/tsdown.config.ts
{packages,plugins}/**/vitest.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each non-CLI package must include a vitest.config.ts for test configuration
Files:
plugins/onerror/vitest.config.ts
tsconfig.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Root tsconfig.json must include all packages in the references array
Files:
tsconfig.json
plugins/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
plugins/*/package.json: Plugin package.json must export TS sources in development ("exports": { ".": "./src/index.ts", "./agent": "./src/agent.ts", "./app": "./src/app.ts", "./package.json": "./package.json" })
Plugin package.json must mirror exports for publish via publishConfig.exports pointing to compiled JS in dist (e.g., "./dist/index.js")
Plugin package.json must include "files": ["dist"]
Plugin package.json must include scripts: build (tsdown), clean (rimraf dist), prepublishOnly (npm run build)
Plugin package.json must include an eggPlugin configuration field
Files:
plugins/onerror/package.json
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to packages/egg/tsconfig.json : packages/egg/tsconfig.json must extend eggjs/tsconfig and enable strict mode
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to packages/egg/tsdown.config.ts : packages/egg must use tsdown configured for unbundled ESM output
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/*/package.json : Plugin package.json must include an eggPlugin configuration field
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins,tools}/**/tsconfig.json : All sub-project tsconfig.json files must include compilerOptions.baseUrl set to "./"
Applied to files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsontools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.json
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files must extend from the root via "extends": "../../tsconfig.json"
Applied to files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsontools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins,tools}/**/tsconfig.json : All sub-project tsconfig.json files must extend the root tsconfig.json using "extends": "../../tsconfig.json"
Applied to files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsontools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to packages/egg/tsconfig.json : packages/egg/tsconfig.json must extend eggjs/tsconfig and enable strict mode
Applied to files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsontools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/*/package.json : Plugin package.json must export TS sources in development ("exports": { ".": "./src/index.ts", "./agent": "./src/agent.ts", "./app": "./src/app.ts", "./package.json": "./package.json" })
Applied to files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsontools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.jsonplugins/onerror/tsdown.config.tsplugins/onerror/package.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/**/tsdown.config.ts : All plugins must use the standard tsdown configuration template (entry: 'src/**/*.ts', unbundle: true, dts: true, exports.devExports: true)
Applied to files:
packages/koa/tsconfig.jsonpackages/supertest/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.jsonplugins/onerror/tsdown.config.tstsconfig.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to tools/egg-bin/tsdown.config.ts : egg-bin tool must use tsdown for TypeScript compilation
Applied to files:
tools/egg-bin/tsconfig.jsonplugins/onerror/tsdown.config.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to packages/egg/tsdown.config.ts : packages/egg must use tsdown configured for unbundled ESM output
Applied to files:
tools/egg-bin/tsconfig.jsonplugins/onerror/tsdown.config.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin (CLI tool) tests must use Mocha
Applied to files:
tools/egg-bin/tsconfig.json
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to {tsconfig.json,packages/*/tsconfig.json} : Enable strict TypeScript mode across all packages
Applied to files:
tools/egg-bin/tsconfig.jsonpackages/utils/tsconfig.jsonplugins/onerror/tsconfig.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins,tools}/**/tsdown.config.ts : Each package must use tsdown for unbundled ESM builds with type declarations (unbundle: true, dts: true)
Applied to files:
tools/egg-bin/tsconfig.jsonplugins/onerror/tsconfig.jsonplugins/onerror/tsdown.config.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/*/tsdown.config.ts : Each package must provide a tsdown.config.ts for unbundled ESM builds
Applied to files:
tools/egg-bin/tsconfig.jsonplugins/onerror/tsconfig.jsonplugins/onerror/tsdown.config.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/*/package.json : Use "workspace:*" for internal workspace dependencies in package.json
Applied to files:
packages/mock/package.json
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to pnpm-workspace.yaml : Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Applied to files:
packages/mock/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to pnpm-workspace.yaml : Define external dependency versions centrally in the catalog section of pnpm-workspace.yaml
Applied to files:
packages/mock/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins,tools}/**/package.json : Individual packages must reference external dependencies via "catalog:" from pnpm-workspace.yaml
Applied to files:
packages/mock/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/vitest.config.ts : Each non-CLI package must include a vitest.config.ts for test configuration
Applied to files:
plugins/onerror/vitest.config.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
Applied to files:
plugins/onerror/vitest.config.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
plugins/onerror/vitest.config.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/*/package.json : Plugin package.json must mirror exports for publish via publishConfig.exports pointing to compiled JS in dist (e.g., "./dist/index.js")
Applied to files:
plugins/onerror/package.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/*/package.json : Plugin package.json must include an eggPlugin configuration field
Applied to files:
plugins/onerror/package.json
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/*/package.json : Plugin package.json must include scripts: build (tsdown), clean (rimraf dist), prepublishOnly (npm run build)
Applied to files:
plugins/onerror/package.json
🧬 Code graph analysis (19)
plugins/onerror/test/fixtures/onerror-customize/config/config.default.js (2)
plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js (2)
err(2-2)err(21-21)plugins/onerror/test/fixtures/onerror-customize/app/controller/user.js (1)
err(2-2)
plugins/onerror/test/fixtures/onerror-customize/app/controller/user.js (1)
plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js (2)
err(2-2)err(21-21)
plugins/onerror/src/lib/utils.ts (5)
plugins/onerror/test/fixtures/onerror-4xx/app/controller/home.js (2)
err(2-2)err(21-21)plugins/onerror/test/fixtures/onerror-custom-template/app/controller/home.js (2)
err(2-2)err(21-21)plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js (2)
err(2-2)err(21-21)plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js (2)
err(2-2)err(21-21)plugins/onerror/test/fixtures/onerror/app/controller/home.js (3)
err(2-2)err(16-16)err(27-27)
plugins/onerror/src/config/config.default.ts (2)
plugins/onerror/src/types.ts (1)
OnerrorConfig(5-5)plugins/onerror/test/fixtures/onerror-custom-template/config/config.default.js (1)
path(1-1)
packages/mock/src/app/extend/agent.ts (1)
packages/egg/src/index.ts (1)
Agent(60-60)
plugins/onerror/src/agent.ts (2)
plugins/onerror/src/app.ts (1)
Boot(17-165)packages/egg/src/lib/core/base_hook_class.ts (1)
agent(31-37)
plugins/onerror/test/fixtures/onerror-custom-template/app/controller/home.js (1)
plugins/onerror/test/fixtures/onerror/app/controller/home.js (3)
err(2-2)err(16-16)err(27-27)
packages/mock/src/lib/parallel/app.ts (1)
packages/utils/src/import.ts (1)
importModule(450-522)
plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js (2)
plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/user.js (1)
err(2-2)plugins/onerror/test/fixtures/onerror/app/controller/home.js (3)
err(2-2)err(16-16)err(27-27)
plugins/onerror/test/onerror.test.ts (3)
packages/mock/test/helper.ts (1)
getFixtures(5-7)packages/mock/src/register.ts (3)
beforeAll(22-28)afterAll(37-45)afterEach(29-36)packages/mock/src/app/extend/application.ts (1)
mm(366-368)
plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/user.js (1)
plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js (2)
err(2-2)err(21-21)
plugins/onerror/test/fixtures/onerror-custom-template/app/controller/user.js (1)
plugins/onerror/test/fixtures/onerror-custom-template/app/controller/home.js (2)
err(2-2)err(21-21)
plugins/onerror/test/fixtures/onerror/app/controller/user.js (2)
plugins/onerror/test/fixtures/onerror-4xx/app/controller/user.js (1)
err(2-2)plugins/onerror/test/fixtures/onerror/app/controller/home.js (3)
err(2-2)err(16-16)err(27-27)
packages/mock/src/app.ts (2)
plugins/onerror/src/agent.ts (1)
Boot(3-16)plugins/onerror/src/app.ts (1)
Boot(17-165)
plugins/onerror/src/types.ts (2)
packages/egg/src/lib/types.ts (1)
EggAppConfig(97-271)plugins/onerror/src/config/config.default.ts (1)
OnerrorConfig(6-26)
packages/mock/src/lib/parallel/agent.ts (1)
packages/utils/src/import.ts (1)
importModule(450-522)
packages/mock/src/app/extend/application.ts (3)
packages/mock/src/index.ts (1)
ApplicationUnittest(97-97)packages/egg/src/index.ts (1)
Application(54-54)packages/mock/src/lib/types.ts (1)
MockOptions(1-50)
plugins/onerror/src/lib/error_view.ts (1)
plugins/onerror/src/lib/utils.ts (1)
detectErrorMessage(4-12)
plugins/onerror/src/app.ts (3)
plugins/onerror/src/lib/utils.ts (4)
detectStatus(14-22)accepts(24-28)isProd(30-32)detectErrorMessage(4-12)plugins/onerror/src/lib/error_view.ts (1)
ErrorView(27-282)plugins/onerror/src/config/config.default.ts (1)
OnerrorConfig(6-26)
🪛 markdownlint-cli2 (0.17.2)
plugins/onerror/CHANGELOG.md
13-13: Bare URL used
(MD034, no-bare-urls)
15-15: Bare URL used
(MD034, no-bare-urls)
42-42: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
45-45: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
46-46: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
48-48: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
51-51: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
52-52: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
54-54: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
55-55: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
58-58: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
61-61: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
62-62: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
64-64: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
67-67: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
68-68: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
70-70: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
73-73: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
74-74: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
77-77: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
78-78: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
79-79: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
81-81: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
84-84: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
86-86: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
89-89: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
91-91: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
94-94: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
96-96: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
99-99: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
102-102: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
105-105: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
107-107: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
110-110: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
112-112: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
115-115: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
117-117: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
120-120: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
122-122: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
125-125: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
127-127: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
130-130: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
132-132: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
135-135: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
137-137: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
140-140: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
141-141: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
143-143: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
146-146: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
148-148: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
151-151: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
152-152: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
154-154: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
157-157: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
159-159: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
161-161: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🪛 GitHub Check: CodeQL
plugins/onerror/src/lib/onerror_page.mustache.html
[warning] 740-740: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
[warning] 741-741: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
DOM text is reinterpreted as HTML without escaping meta-characters.
plugins/onerror/src/lib/error_view.ts
[failure] 202-202: Incomplete string escaping or encoding
This replaces only the first occurrence of /\|//.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Test egg-bin (macos-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (macos-latest, 24)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 22)
- GitHub Check: Test packages (macos-latest, 24)
- GitHub Check: Test packages (macos-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 24)
Move @eggjs/onerror plugin from separate repository to plugins/onerror. Configure for monorepo with workspace dependencies and standard tsdown build. - Add onerror plugin to plugins directory - Configure package.json with workspace and catalog dependencies - Add standard tsdown.config.ts following plugin conventions - Add vitest.config.ts for test configuration - Update root tsconfig.json references - Add cookie, koa-onerror, mustache, stack-trace to pnpm catalog - Add type definitions for mustache and stack-trace - Fix HTML syntax error in mustache template - Remove incorrectly created packages/egg/plugins/onerror directory 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
plugins/onerror/test/fixtures/onerror-custom-template/template.mustache (2)
114-114: Fix malformed HTML closing tag.There's a malformed closing tag
</<div>that will cause invalid HTML output.- </<div> + </div>
153-170: Security: Avoid implicit globals and usetextContentinstead ofinnerHTMLfor safer DOM manipulation.The code creates
$frameContextwithout proper declaration and uses.innerHTMLwhich can lead to XSS vulnerabilities when handling user-controlled error data. UsetextContentfor safer rendering.- $frameContext = frame.querySelector('.frame-context'); - var $context = $frameContext.innerHTML; + const $frameContext = frame.querySelector('.frame-context'); + let $context = $frameContext.textContent || ''; $context = $context.trim().length === 0 ? 'Missing stack frames' : $context; var $line = $frameContext.getAttribute('data-line'); var $start = $frameContext.getAttribute('data-start'); var $file = $frameContext.getAttribute('data-file'); var $method = $frameContext.getAttribute('data-method'); var $lineColumn = $frameContext.getAttribute('data-line-column'); var $language = $frameContext.getAttribute('data-extname') || 'js'; $('#code-drop').parentNode.setAttribute('data-line', $line); $('#code-drop').parentNode.setAttribute('data-start', $start); $('#code-drop').parentNode.setAttribute('data-language', $language); $('#code-drop').parentNode.setAttribute('data-line-offset', (Number($start) - 1)); $('#code-drop').setAttribute('class', 'language-' + $language); - $('#code-drop').innerHTML = $context; - $('#frame-file').innerHTML = $file; - $('#frame-method').innerHTML = $method + '' + $lineColumn; + $('#code-drop').textContent = $context; + $('#frame-file').textContent = $file; + $('#frame-method').textContent = $method + '' + $lineColumn;plugins/onerror/src/app.ts (1)
136-139: Don’t overwrite falsy error fields; check presence instead.Use an existence check so
0,false, or''aren’t clobbered.- for (const key in err) { - if (!errorJson[key]) { - errorJson[key] = (err as any)[key]; - } - } + for (const key in err) { + if (!(key in errorJson)) { + errorJson[key] = (err as any)[key]; + } + }plugins/onerror/src/lib/error_view.ts (2)
274-276: Simplify the function check.
inis redundant here.- if ('dumpConfigToObject' in this.app && typeof this.app.dumpConfigToObject === 'function') { + if (typeof this.app.dumpConfigToObject === 'function') {
127-142: Optional: make source loading async to avoid sync I/O in request path.Converting
getFrameSource,parseError, andtoHTMLtoasyncwithfs.promiseswill prevent event‑loop blocking when rendering the dev HTML page.Also applies to: 147-155, 52-65
🧹 Nitpick comments (27)
plugins/onerror/test/fixtures/custom-listener-onerror/app/router.js (1)
4-4: Nit: consider nullish coalescing to avoid treating empty string as “missing”.Only if you want
?name=to keep an empty name instead of defaulting to "Error".- err.name = ctx.query.name || 'Error'; + err.name = ctx.query?.name ?? 'Error';packages/egg/test/cluster1/cluster-client.test.ts (2)
19-21: Prefer a narrow cast over @ts-expect-error for symbol-indexed accessUsing @ts-expect-error makes the test brittle if types are later added. A targeted cast avoids disabling the checker and keeps runtime identical.
- // @ts-expect-error registryClient no type definition - const agentInnerClient = app.agent.registryClient[innerClient]; + const agentInnerClient = + (app.agent.registryClient as any)[innerClient] as { + _realClient: { closed: boolean }; + };- // @ts-expect-error registryClient no type definition - const agentInnerClient = app.agent.registryClient[innerClient]; + const agentInnerClient = + (app.agent.registryClient as any)[innerClient] as { + _realClient: { closed: boolean }; + };Also applies to: 61-63
22-22: Unify mm.restore usage (await vs non‑await)First block awaits mm.restore(), second does not. Make them consistent to avoid subtle teardown ordering issues.
- mm.restore(); + await mm.restore();Please confirm whether
mm.restore()is sync or async in@eggjs/mock; awaiting a sync function is harmless, but consistency helps.Also applies to: 64-64
packages/egg/test/app/extend/agent.test.ts (2)
19-21: Replace ts-expect-error with test-only type augmentation (cleaner, future-proof)Avoid locking-in an expected error; augment Agent in test types to keep type-safety and readability.
Apply this diff in this file (remove the ts-expect-error comments):
- // @ts-expect-error dataService no type definition const dataService = app.agent.dataService; ... - // @ts-expect-error dataServiceAsync no type definition const dataServiceAsync = app.agent.dataServiceAsync;Add a test-only ambient declaration (new file) to type the properties without leaking to prod:
// packages/egg/test/types/agent-augment.d.ts import 'egg'; declare module 'egg' { interface Agent { // Keep broad on purpose; refine later if real types are exposed dataService: any; dataServiceAsync: any; } }Also applies to: 35-37
5-5: Prefer extensionless import (drop .ts)packages/tsconfig/tsconfig.json sets "module" and "moduleResolution" to "NodeNext"; createApp is exported at packages/egg/test/utils.ts:47 — change the import to avoid NodeNext ESM resolution pitfalls.
Apply this diff:
-import { restore, createApp, type MockApplication } from '../../utils.ts'; +import { restore, createApp, type MockApplication } from '../../utils';plugins/onerror/CHANGELOG.md (2)
13-13: Consider wrapping bare URLs in angle brackets for better Markdown formatting.According to Markdown best practices, bare URLs should be wrapped in angle brackets to ensure proper rendering across different parsers.
-part of https://github.com/eggjs/egg/issues/3644 +part of <https://github.com/eggjs/egg/issues/3644> -https://github.com/eggjs/egg/issues/5257 +<https://github.com/eggjs/egg/issues/5257>Also applies to: 15-15
42-162: Consider standardizing the changelog heading format.The changelog uses a mix of ATX (
##) and Setext (===) heading styles. For consistency, consider using ATX style throughout.Apply this diff to standardize heading format:
-2.2.0 / 2022-12-11 -================== +## 2.2.0 / 2022-12-11 -2.1.1 / 2022-08-18 -================== +## 2.1.1 / 2022-08-18 # Continue this pattern for all remaining version headings...plugins/onerror/test/fixtures/onerror-ctx-error/app/middleware/trigger.js (1)
1-7: Missing semicolon after thectx.userIdlog statement.Line 5 is missing a semicolon at the end. While JavaScript's automatic semicolon insertion will handle this, it's better to be explicit for consistency.
- ctx.logger.info('%s', ctx.userId) + ctx.logger.info('%s', ctx.userId);plugins/onerror/README.md (1)
53-53: Use more descriptive link text for better accessibility.The link text "here" is not descriptive. Screen readers and users benefit from link text that describes the destination.
-Please open an issue [here](https://github.com/eggjs/egg/issues). +Please open an issue on [the Egg.js GitHub repository](https://github.com/eggjs/egg/issues).plugins/onerror/src/app.ts (3)
110-115: Guard STATUS_CODES lookup for non‑standard 5xx.Fallback avoids
undefinedmessages for uncommon codes.- errorJson = { - code, - // don't respond any error message in production env - message: http.STATUS_CODES[status], - }; + errorJson = { + code, + // don't respond any error message in production env + message: http.STATUS_CODES[status] || 'Internal Server Error', + };
80-87: Set explicit Content‑Type for string bodies.Be explicit to avoid relying on framework defaults.
ctx.status = 500; ctx.body = `<h2>Internal Server Error, real status: ${status}</h2>`; + ctx.type = 'html'; return;ctx.status = status; ctx.body = `<h2>${status} ${http.STATUS_CODES[status]}</h2>`; + ctx.type = 'html'; return;if (app.config.env === 'unittest') { ctx.status = status; ctx.body = `${err.name}: ${err.message}\n${err.stack}`; + ctx.type = 'text/plain'; return; }Also applies to: 90-94
147-151: Drop unnecessary .call; pass args directly.No
thisusage injson().- errorOptions.json!.call(ctx, err, ctx); + errorOptions.json!(err, ctx);plugins/onerror/test/onerror.test.ts (3)
29-31: Fix test title grammar.-it('should set app.config.onerror.errorPageUrl work', () => { +it('should have app.config.onerror.errorPageUrl configured', () => {
115-120: Typo: “accpets” → “accepts”.-it('should support custom accpets return err.stack', () => { +it('should support custom accepts return err.stack', () => {
324-326: Remove duplicate assignment (app = app = …).- app = app = mm.app({ + app = mm.app({Also applies to: 344-346
plugins/onerror/src/lib/error_view.ts (3)
15-15: Fix regex to strip all leading slashes/backslashes (addresses CodeQL finding).-const startingSlashRegex = /\\|\//; +const startingSlashRegex = /^[\\/]+/;
137-141: Avoid undefined when source file is missing.Provide a safe default for the current line.
- line: lines[lineNumber - 1], + line: lines[lineNumber - 1] ?? '',
36-36: Don’t expose Authorization header on the error page.Extend the filter list.
- _filterHeaders = [ 'cookie', 'connection' ]; + _filterHeaders = [ 'cookie', 'connection', 'authorization' ];packages/mock/src/lib/mock_agent.ts (3)
3-8: Use type-only imports for Dispatcher and HttpClient to avoid runtime imports.This prevents unnecessary values from being loaded at runtime and aligns with ESM + TS best practices.
-import { - MockAgent, - setGlobalDispatcher, - getGlobalDispatcher, - Dispatcher, - HttpClient, -} from 'urllib'; +import { MockAgent, setGlobalDispatcher, getGlobalDispatcher } from 'urllib'; +import type { Dispatcher, HttpClient } from 'urllib';
12-16: Type the globals as possibly undefined to match usage.You guard on “falsy” for __globalDispatcher and store values lazily; TS should reflect that to avoid “always truthy” checks and enable future sentinels if needed.
Suggested change:
declare namespace globalThis { let __mockAgent: MockAgent | null; - let __globalDispatcher: Dispatcher; - let __httpClientDispatchers: Map<HttpClient, Dispatcher>; + let __globalDispatcher: Dispatcher | undefined; + let __httpClientDispatchers: Map<HttpClient, Dispatcher>; }
1-1: Rename file to hyphen-case per repo guidelines.Suggest: mock-agent.ts
packages/mock/src/lib/parallel/app.ts (2)
23-24: Make _instance optional (or definite-assignment) to satisfy strictPropertyInitialization.Prevents unsafe access before init and avoids relying on compiler flags.
- _instance: EggApplication; + _instance?: EggApplication;
5-5: Avoid runtime import of 'egg' when only using types.Switch to type-only import and use typeof import(...) to keep ESM tree-shakeable and eliminate an unnecessary module load.
-import { Application as EggApplication } from 'egg'; +import type { Application as EggApplication } from 'egg'; @@ - const { Application }: { Application: typeof EggApplication } = await importModule( + const { Application }: { Application: typeof import('egg').Application } = await importModule( this.options.framework );Also applies to: 44-48
packages/mock/src/app/extend/application.ts (4)
209-212: Runtime type assertion for mocked member — LGTM.Tiny nit: consider including the containing type/class name in the error for easier debugging.
375-383: Deprecated mockUrllib with clear message — LGTM.Consider adding a one-release warning gate in CI to catch new usages.
461-465: Preserve original logger.log call after capture — LGTM.If custom loggers override log signature, consider guarding arity with typeof checks. Non-blocking.
468-472: Handle non-file loggers in __checkExpectLog to avoid fs.readFileSync(undefined).Add a guard to give a clearer error when logger has no file transport and mockLog() wasn’t called.
const filepath = logger.options.file; - let content; + let content; if ('_mockLogs' in logger && logger._mockLogs) { content = (logger._mockLogs as string[]).join('\n'); } else { - content = fs.readFileSync(filepath, 'utf8'); + assert( + filepath, + 'expectLog/notExpectLog requires a file-based logger or call app.mockLog(logger) first' + ); + content = fs.readFileSync(filepath, 'utf8'); }Also applies to: 494-497, 499-502
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (76)
packages/egg/test/app/extend/agent.test.ts(2 hunks)packages/egg/test/app/extend/context.test.ts(2 hunks)packages/egg/test/cluster1/cluster-client.test.ts(2 hunks)packages/egg/test/lib/core/httpclient.test.ts(1 hunks)packages/egg/test/lib/core/loader/load_boot.test.ts(2 hunks)packages/koa/tsconfig.json(1 hunks)packages/mock/package.json(1 hunks)packages/mock/src/app.ts(1 hunks)packages/mock/src/app/extend/agent.ts(2 hunks)packages/mock/src/app/extend/application.ts(13 hunks)packages/mock/src/app/middleware/cluster_app_mock.ts(1 hunks)packages/mock/src/lib/context.ts(0 hunks)packages/mock/src/lib/mock_agent.ts(3 hunks)packages/mock/src/lib/parallel/agent.ts(3 hunks)packages/mock/src/lib/parallel/app.ts(3 hunks)packages/mock/test/app.test.ts(1 hunks)packages/supertest/tsconfig.json(1 hunks)packages/utils/tsconfig.json(1 hunks)plugins/onerror/CHANGELOG.md(1 hunks)plugins/onerror/LICENSE(1 hunks)plugins/onerror/README.md(1 hunks)plugins/onerror/package.json(1 hunks)plugins/onerror/src/agent.ts(1 hunks)plugins/onerror/src/app.ts(1 hunks)plugins/onerror/src/config/config.default.ts(1 hunks)plugins/onerror/src/index.ts(1 hunks)plugins/onerror/src/lib/error_view.ts(1 hunks)plugins/onerror/src/lib/onerror_page.mustache.html(1 hunks)plugins/onerror/src/lib/utils.ts(1 hunks)plugins/onerror/src/types.ts(1 hunks)plugins/onerror/test/fixtures/agent-error/agent.js(1 hunks)plugins/onerror/test/fixtures/agent-error/package.json(1 hunks)plugins/onerror/test/fixtures/custom-listener-onerror/app/router.js(1 hunks)plugins/onerror/test/fixtures/custom-listener-onerror/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/custom-listener-onerror/package.json(1 hunks)plugins/onerror/test/fixtures/mock-test-error/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-4xx/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/app/extend/context.js(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/app/middleware/trigger.js(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-ctx-error/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-custom-500/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-500/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-500/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-custom-template/template.mustache(1 hunks)plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-customize/package.json(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror-no-errorpage/package.json(1 hunks)plugins/onerror/test/fixtures/onerror/app/controller/home.js(1 hunks)plugins/onerror/test/fixtures/onerror/app/controller/user.js(1 hunks)plugins/onerror/test/fixtures/onerror/app/router.js(1 hunks)plugins/onerror/test/fixtures/onerror/config/config.default.js(1 hunks)plugins/onerror/test/fixtures/onerror/package.json(1 hunks)plugins/onerror/test/onerror.test.ts(1 hunks)plugins/onerror/tsconfig.json(1 hunks)plugins/onerror/tsdown.config.ts(1 hunks)plugins/onerror/vitest.config.ts(1 hunks)pnpm-workspace.yaml(4 hunks)tools/egg-bin/tsconfig.json(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/mock/src/lib/context.ts
✅ Files skipped from review due to trivial changes (4)
- packages/mock/test/app.test.ts
- packages/egg/test/app/extend/context.test.ts
- packages/egg/test/lib/core/loader/load_boot.test.ts
- packages/egg/test/lib/core/httpclient.test.ts
🚧 Files skipped from review as they are similar to previous changes (57)
- plugins/onerror/src/index.ts
- plugins/onerror/test/fixtures/onerror-customize/app/controller/home.js
- plugins/onerror/test/fixtures/onerror-customize/app/controller/user.js
- plugins/onerror/test/fixtures/onerror/package.json
- plugins/onerror/tsconfig.json
- plugins/onerror/test/fixtures/onerror-4xx/app/controller/home.js
- plugins/onerror/test/fixtures/onerror-4xx/app/router.js
- plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/home.js
- plugins/onerror/test/fixtures/agent-error/package.json
- packages/koa/tsconfig.json
- plugins/onerror/test/fixtures/custom-listener-onerror/package.json
- plugins/onerror/test/fixtures/onerror-custom-template/app/controller/home.js
- packages/mock/package.json
- plugins/onerror/test/fixtures/custom-listener-onerror/config/config.default.js
- plugins/onerror/test/fixtures/onerror-no-errorpage/app/controller/user.js
- plugins/onerror/test/fixtures/onerror/app/controller/home.js
- plugins/onerror/src/config/config.default.ts
- plugins/onerror/test/fixtures/onerror-custom-500/app/router.js
- plugins/onerror/test/fixtures/onerror/config/config.default.js
- packages/mock/src/app/middleware/cluster_app_mock.ts
- plugins/onerror/test/fixtures/onerror-custom-template/config/config.default.js
- plugins/onerror/test/fixtures/onerror-custom-500/config/config.default.js
- plugins/onerror/vitest.config.ts
- plugins/onerror/test/fixtures/onerror-no-errorpage/package.json
- plugins/onerror/test/fixtures/onerror-ctx-error/package.json
- plugins/onerror/src/lib/onerror_page.mustache.html
- plugins/onerror/test/fixtures/onerror-customize/config/config.default.js
- plugins/onerror/test/fixtures/onerror-custom-500/package.json
- tools/egg-bin/tsconfig.json
- plugins/onerror/test/fixtures/onerror-customize/package.json
- plugins/onerror/src/lib/utils.ts
- pnpm-workspace.yaml
- plugins/onerror/test/fixtures/onerror-4xx/config/config.default.js
- plugins/onerror/test/fixtures/onerror/app/controller/user.js
- plugins/onerror/test/fixtures/onerror/app/router.js
- packages/mock/src/app.ts
- packages/utils/tsconfig.json
- plugins/onerror/test/fixtures/agent-error/agent.js
- plugins/onerror/test/fixtures/onerror-no-errorpage/app/router.js
- plugins/onerror/package.json
- plugins/onerror/test/fixtures/onerror-no-errorpage/config/config.default.js
- plugins/onerror/test/fixtures/onerror-4xx/package.json
- plugins/onerror/test/fixtures/onerror-ctx-error/config/config.default.js
- plugins/onerror/tsdown.config.ts
- packages/mock/src/lib/parallel/agent.ts
- plugins/onerror/test/fixtures/onerror-custom-template/package.json
- packages/supertest/tsconfig.json
- plugins/onerror/src/agent.ts
- plugins/onerror/test/fixtures/onerror-custom-template/app/controller/user.js
- plugins/onerror/test/fixtures/mock-test-error/package.json
- plugins/onerror/src/types.ts
- plugins/onerror/LICENSE
- plugins/onerror/test/fixtures/onerror-customize/app/router.js
- tsconfig.json
- plugins/onerror/test/fixtures/onerror-custom-template/app/router.js
- packages/mock/src/app/extend/agent.ts
- plugins/onerror/test/fixtures/onerror-4xx/app/controller/user.js
🧰 Additional context used
📓 Path-based instructions (6)
packages/**/test/**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/test/**/*.test.ts: Name test files as test/**/*.test.ts and run them with Vitest
Use import { describe, it } from 'vitest' in tests
Use Node.js built-in assert module for test assertions
Files:
packages/egg/test/app/extend/agent.test.tspackages/egg/test/cluster1/cluster-client.test.ts
{packages,plugins}/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins}/**/test/**/*.test.ts: Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
Use Node.js built-in assert for assertions in Vitest tests
Files:
packages/egg/test/app/extend/agent.test.tsplugins/onerror/test/onerror.test.tspackages/egg/test/cluster1/cluster-client.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
packages/egg/test/app/extend/agent.test.tsplugins/onerror/src/lib/error_view.tspackages/mock/src/lib/parallel/app.tsplugins/onerror/test/onerror.test.tspackages/mock/src/lib/mock_agent.tspackages/egg/test/cluster1/cluster-client.test.tsplugins/onerror/src/app.tspackages/mock/src/app/extend/application.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suites
Files:
packages/egg/test/app/extend/agent.test.tsplugins/onerror/test/onerror.test.tspackages/egg/test/cluster1/cluster-client.test.ts
**/test/fixtures/**
📄 CodeRabbit inference engine (AGENTS.md)
Put reusable test data under test/fixtures/
Files:
plugins/onerror/test/fixtures/custom-listener-onerror/app/router.jsplugins/onerror/test/fixtures/onerror-ctx-error/app/middleware/trigger.jsplugins/onerror/test/fixtures/onerror-ctx-error/app/extend/context.jsplugins/onerror/test/fixtures/onerror-custom-template/template.mustache
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use TypeScript for all source files in packages (no JavaScript source files)
Files:
packages/mock/src/lib/parallel/app.tspackages/mock/src/lib/mock_agent.tspackages/mock/src/app/extend/application.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to plugins/*/package.json : Plugin package.json must include an eggPlugin configuration field
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
packages/egg/test/app/extend/agent.test.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Use Node.js built-in assert for assertions in Vitest tests
Applied to files:
packages/egg/test/app/extend/agent.test.tsplugins/onerror/test/onerror.test.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin (CLI tool) tests must use Mocha
Applied to files:
packages/egg/test/app/extend/agent.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps
Applied to files:
packages/egg/test/app/extend/agent.test.tsplugins/onerror/test/onerror.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
packages/egg/test/app/extend/agent.test.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
Applied to files:
packages/egg/test/app/extend/agent.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
packages/egg/test/app/extend/agent.test.ts
🧬 Code graph analysis (6)
packages/egg/test/app/extend/agent.test.ts (1)
packages/egg/test/utils.ts (1)
app(40-45)
plugins/onerror/src/lib/error_view.ts (1)
plugins/onerror/src/lib/utils.ts (1)
detectErrorMessage(4-12)
packages/mock/src/lib/parallel/app.ts (1)
packages/utils/src/import.ts (1)
importModule(450-522)
packages/mock/src/lib/mock_agent.ts (1)
packages/mock/src/lib/parallel/agent.ts (2)
MockAgent(18-128)debug(79-91)
plugins/onerror/src/app.ts (3)
plugins/onerror/src/lib/utils.ts (4)
detectStatus(14-22)accepts(24-28)isProd(30-32)detectErrorMessage(4-12)plugins/onerror/src/lib/error_view.ts (1)
ErrorView(27-282)plugins/onerror/src/config/config.default.ts (1)
OnerrorConfig(6-26)
packages/mock/src/app/extend/application.ts (4)
packages/egg/src/index.ts (1)
Application(54-54)packages/mock/src/lib/types.ts (5)
MockOptions(1-50)MockHttpClientMethod(84-84)MockResultOptions(82-82)MockResultFunction(83-83)MockAgent(88-88)packages/mock/src/lib/mock_httpclient.ts (3)
MockHttpClientMethod(205-205)MockResultOptions(7-29)MockResultFunction(42-45)packages/mock/src/lib/parallel/agent.ts (1)
MockAgent(18-128)
🪛 GitHub Check: CodeQL
plugins/onerror/src/lib/error_view.ts
[failure] 202-202: Incomplete string escaping or encoding
This replaces only the first occurrence of /\|//.
🪛 markdownlint-cli2 (0.18.1)
plugins/onerror/CHANGELOG.md
13-13: Bare URL used
(MD034, no-bare-urls)
15-15: Bare URL used
(MD034, no-bare-urls)
42-42: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
45-45: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
46-46: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
48-48: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
51-51: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
52-52: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
54-54: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
55-55: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
58-58: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
61-61: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
62-62: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
64-64: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
67-67: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
68-68: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
70-70: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
73-73: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
74-74: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
77-77: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
78-78: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
79-79: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
81-81: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
84-84: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
86-86: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
89-89: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
91-91: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
94-94: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
96-96: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
99-99: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
102-102: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
105-105: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
107-107: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
110-110: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
112-112: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
115-115: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
117-117: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
120-120: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
122-122: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
125-125: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
127-127: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
130-130: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
132-132: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
135-135: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
137-137: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
140-140: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
141-141: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
143-143: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
146-146: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
148-148: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
151-151: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
152-152: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
154-154: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
157-157: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
159-159: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
161-161: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
plugins/onerror/README.md
53-53: Link text should be descriptive
(MD059, descriptive-link-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (21)
plugins/onerror/test/fixtures/custom-listener-onerror/app/router.js (1)
1-7: LGTM — minimal, focused fixture to trigger onerror with customizable name.Fits test/fixtures conventions and should exercise the plugin as intended.
packages/egg/test/app/extend/agent.test.ts (2)
25-34: LGTM: happy path and async path assertions are clearGood coverage of sync/async instance creation and config checks with strict assert usage.
Also applies to: 45-49
41-43: LGTM: correct use of assert.throws for the sync-only APIRegex message check is appropriate. If
createInstanceever becomes async, switch this toassert.rejects.Confirm API contract in the onerror plugin docs/spec to ensure
createInstanceis guaranteed to throw synchronously.plugins/onerror/test/fixtures/onerror-ctx-error/app/extend/context.js (1)
2-4: LGTM! Clever test fixture for error handling.The getter that throws an error is a good pattern for testing error scenarios in the onerror plugin. This ensures the error handling paths are exercised during tests.
packages/mock/src/lib/mock_agent.ts (3)
57-61: Restore logs look good.Clear, useful context about how many clients will be restored.
61-64: Loop variable naming/readability improved.This is clearer than generic names and makes the intent obvious.
21-21: Explicit return type is good — no void callsites found.rg matches: packages/mock/src/app/extend/agent.ts:40, packages/mock/src/app/extend/application.ts:391, packages/mock/src/lib/mock_httpclient.ts:146–147 — all usages consume the returned MockAgent (returned or chained). No action required.
packages/mock/src/app/extend/application.ts (14)
53-55: MockContext now extends egg Context — LGTM.
85-89: Typed mockContext signature and return — LGTM.
101-104: Guard for optional _customMockContext — LGTM.
130-141: mockContextScope API typing — LGTM.
191-195: mockServiceError typing and overload behavior — LGTM.
218-221: Async vs sync origin tracking — LGTM.
228-233: Async wrapper preserves this/args — LGTM.
324-329: Header getter patch is scoped and case-insensitive — LGTM.
348-355: Alias: mockHttpclient -> mockHttpClient — LGTM.
360-371: mockHttpClient wiring to factory with cached instance — LGTM.
390-392: Typed mockAgent() return — LGTM.
9-16: Imports look good; runtime dependencies declared.
packages/mock/package.json declares "egg-logger" and "urllib" under dependencies.
301-312: createContext monkey-patch in mockCookies — teardown covered.afterEach(mm.restore) is present in packages/mock/test/mock_cookies.test.ts and packages/mock/test/mock_headers.test.ts, so the createContext/cookies monkey-patch is restored; no change required.
57-64: Hold approval — verification found residual EggCore/EggContext/EggCoreOptions usages; verify.rg found occurrences in multiple files (examples: packages/utils/src/plugin.ts, packages/egg/src/lib/egg.ts, packages/core/src/egg.ts and related tests/fixtures). Remove/replace these stale types or confirm they are intentionally retained and re-run the verification script.
1 participant
Move @eggjs/onerror plugin from separate repository to plugins/onerror. Configure for monorepo with workspace dependencies and standard tsdown build.
🤖 Generated with Claude Code
Summary by CodeRabbit