Skip to content

Adds error options to AppError class #191

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
w3nl merged 1 commit into from
Oct 16, 2025
Merged

Adds error options to AppError class #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ You can send:
* value (type: mixed)
* type (String|Array|Object|Number|Boolean|Date|...)
* me (current method, no validation on the value)
* options (ErrorOptions, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error)

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

Clarify that options must be an object (not null) and that when provided with a cause, it will be available on error.cause. Consider adding a short example: new AppError({ message: '...', options: { cause: new Error('reason') } }) -> error.cause.message === 'reason'.

Copilot uses AI. Check for mistakes.

@coderabbitai coderabbitai Bot Oct 16, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix markdown formatting issues.

The line has two formatting issues flagged by markdownlint:

  1. List indentation is incorrect (should be 0 spaces, currently 1)
  2. Bare URL should be formatted as a proper link

Apply this diff to fix the formatting:

- * options (ErrorOptions, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error)
+* options (ErrorOptions, see <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error>)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* options (ErrorOptions, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error)
* options (ErrorOptions, see <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error>)
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

29-29: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)

29-29: Bare URL used

(MD034, no-bare-urls)

🤖 Prompt for AI Agents 
In README.md around line 29, the list item has incorrect indentation and a bare
URL; fix by removing the leading space so the asterisk is at column 0 and
replace the raw URL with a proper markdown link (e.g. ErrorOptions linking to
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error)
so the line reads a properly indented list item with a formatted link.


It validate the values by this schema:
```javascript
Expand Down
2,132 changes: 296 additions & 1,836 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

0 src/__tests__/access-error.unit.js → src/__tests__/access-error.test.js
View file
Open in desktop
File renamed without changes.
9 changes: 6 additions & 3 deletions src/__tests__/app-error.unit.js → src/__tests__/app-error.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, it } from 'node:test'

Check warning on line 1 in src/__tests__/app-error.test.js

View workflow job for this annotation

GitHub Actions / build (20.x) 

The 'test.describe' is still an experimental feature and is not supported until Node.js 22.0.0 (backported: ^20.13.0). The configured version range is '>= 20'

Check warning on line 1 in src/__tests__/app-error.test.js

View workflow job for this annotation

GitHub Actions / build (21.x) 

The 'test.describe' is still an experimental feature and is not supported until Node.js 22.0.0 (backported: ^20.13.0). The configured version range is '>= 20'

Check warning on line 1 in src/__tests__/app-error.test.js

View workflow job for this annotation

GitHub Actions / build (22.x) 

The 'test.describe' is still an experimental feature and is not supported until Node.js 22.0.0 (backported: ^20.13.0). The configured version range is '>= 20'

Check warning on line 1 in src/__tests__/app-error.test.js

View workflow job for this annotation

GitHub Actions / build (23.x) 

The 'test.describe' is still an experimental feature and is not supported until Node.js 22.0.0 (backported: ^20.13.0). The configured version range is '>= 20'
import assert from 'node:assert'
import { AppError } from '../index.js'

Expand Down Expand Up @@ -27,7 +27,8 @@
value: null,
type: null,
message: 'Example text',
me: null
me: null,
options: null
Comment on lines +30 to +31

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

options: null is not a valid ErrorOptions per the ES2022 Error constructor and may cause a TypeError in modern runtimes; use undefined or omit the property instead. Replace with options: undefined, or remove the options field entirely.

Suggested change
me: null,
options: null
me: null

Copilot uses AI. Check for mistakes.
})

assert.deepEqual(error instanceof AppError, true)
Expand All @@ -47,7 +48,8 @@
value: undefined,
type: undefined,
message: 'Example text',
me: undefined
me: undefined,
options: undefined
})

assert.deepEqual(error instanceof AppError, true)
Expand All @@ -67,7 +69,8 @@
value: 'test',
type: String,
message: 'Example text',
me: AppError
me: AppError,
options: { cause: new Error('Root cause') }
})
Comment on lines 69 to 74

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

The new behavior (options.cause propagation) isn't asserted. Add expectations to verify that 'error.cause' is set when provided and absent otherwise, e.g., 'assert.equal(error.cause instanceof Error, true)' and 'assert.equal(error.cause.message, 'Root cause')' for this case, and 'assert.equal(error.cause, undefined)' in the null/undefined options cases.

Copilot generated this review using guidance from repository custom instructions.

assert.deepEqual(error instanceof AppError, true)
Comment on lines +72 to 76

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

The test does not assert that the cause is propagated, which is the core behavior introduced by this PR. Add an assertion such as assert.equal(error.cause?.message, 'Root cause') and, in the tests where options is omitted/undefined, assert that error.cause is undefined.

Copilot generated this review using guidance from repository custom instructions.
Expand Down
0 src/__tests__/authentication-error.unit.js → src/__tests__/authentication-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/no-content-error.unit.js → src/__tests__/no-content-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/not-found-error.unit.js → src/__tests__/not-found-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/not-implemented-error.unit.js → src/__tests__/not-implemented-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/processing-error.unit.js → src/__tests__/processing-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/rate-limit-error.unit.js → src/__tests__/rate-limit-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/response-error.unit.js → src/__tests__/response-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/server-error.unit.js → src/__tests__/server-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/timeout-error.unit.js → src/__tests__/timeout-error.test.js
View file
Open in desktop
File renamed without changes.
0 src/__tests__/validation-error.unit.js → src/__tests__/validation-error.test.js
View file
Open in desktop
File renamed without changes.
12 changes: 10 additions & 2 deletions src/app-error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ import errorSchema from './schemas/error.js'

const validator = new Validator(errorSchema)

/**
* Options for the native Error constructor (ES2022).
* @typedef {object} ErrorOptions
* @property {unknown} [cause]
*/

Comment on lines +7 to +12

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

This local JSDoc typedef name 'ErrorOptions' conflicts with the global ES2022 'ErrorOptions' type in TypeScript/checkJs environments, causing duplicate identifier errors. Remove this typedef and reference the built-in 'ErrorOptions' in JSDoc, or rename it (e.g., 'AppErrorOptions') and update the constructor JSDoc to use that new name.

Copilot generated this review using guidance from repository custom instructions.
/**
* @typedef {object} ErrorValues
* @property {string} name
Expand All @@ -24,9 +30,11 @@ export default (error = Error) =>
* @param {object=} error.type
* @param {string} error.message
* @param {object=} error.me
* @param {ErrorOptions=} error.options

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

JSDoc states 'options' is of type ErrorOptions (object), but tests pass null. Clarify the accepted types to avoid confusion and checkJs complaints. Suggest updating to: '@param {ErrorOptions|undefined|null=} error.options'.

Suggested change
* @param {ErrorOptions=} error.options
* @param {ErrorOptions|undefined|null=} error.options

Copilot uses AI. Check for mistakes.
*/
constructor ({ value = null, type = null, message, me = null }) {
super(message)
constructor ({ value = null, type = null, message, me = null, options = null }) {
const opts = options == null ? undefined : options
super(message, opts)
Comment on lines +36 to +37

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Avoid loose equality and the temporary variable; use nullish coalescing inline. For example: super(message, options ?? undefined). This keeps the intent (treat null/undefined as absent) while remaining concise and avoiding ==.

Suggested change
const opts = options == null ? undefined : options
super(message, opts)
super(message, options ?? undefined)

Copilot uses AI. Check for mistakes.

if (Error.captureStackTrace) {
Error.captureStackTrace(this, AppError)
Expand Down
Loading