feat: remove http-assert #18
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes remove all references to the external Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Context as Context.assert
participant Error as createError
Caller->>Context: call assert(value, status, errorMessage?, errorProps?)
alt Value is truthy
Context->>Caller: continue execution
else Value is falsy
Context->>Error: call createError(status, errorMessage, errorProps)
Error-->>Context: error object
Context-->>Caller: throw error
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
test/context/assert.test.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eggache". (The package "eslint-plugin-eggache" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eggache" was referenced from the config file in ".eslintrc » eslint-config-egg/typescript » ./index.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request removes the dependency on http-assert, updating the assertion implementation and the related documentation accordingly.
- Removed the import and usage of http-assert in src/context.ts, replacing it with a direct error throw using createError.
- Updated the API docs in docs/api/context.md to eliminate outdated references to http-assert.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/context.ts | Replaced http-assert with a custom assert implementation using createError. |
| docs/api/context.md | Removed outdated documentation referencing http-assert. |
commit: |
|
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: npm/[email protected] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/context.ts (2)
83-86: Implementation simplified but watch for undefined parametersThe implementation has been streamlined by removing the dependency on
http-assert, which aligns with the PR objective. However, the direct use of non-null assertions (!) on parameters assumes they will be defined when needed.Consider adding null checks or default values to make the code more robust:
- throw createError(status!, msgOrOptions!, opts!); + throw createError( + status || 500, + typeof msgOrOptions === 'string' || msgOrOptions ? msgOrOptions : 'Assertion failed', + opts + );
71-79: JSDoc parameters don't match signature and implementationThe JSDoc parameter documentation doesn't accurately reflect the method signature and implementation. The
@param {String} optscomment is incorrect as the opts parameter is actuallyRecord<string, any>.Update the JSDoc to match the method signature:
/** * Similar to .throw(), adds assertion. * * this.assert(this.user, 401, 'Please login!'); * * @param {Mixed} value * @param {Number} status - * @param {String} opts + * @param {String|Object} msgOrOptions - Error message or options + * @param {Object} [opts] - Error options */🧰 Tools
🪛 GitHub Actions: Node.js CI
[warning] 79-79: http-errors deprecated non-error status code; use only 4xx or 5xx status codes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/api/context.md(0 hunks)package.json(0 hunks)src/context.ts(1 hunks)
💤 Files with no reviewable changes (2)
- docs/api/context.md
- package.json
🧰 Additional context used
🪛 GitHub Actions: Node.js CI
src/context.ts
[warning] 79-79: http-errors deprecated non-error status code; use only 4xx or 5xx status codes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18 +/- ##
==========================================
- Coverage 98.45% 98.35% -0.10%
==========================================
Files 6 6
Lines 1939 1946 +7
Branches 368 373 +5
==========================================
+ Hits 1909 1914 +5
- Misses 30 32 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
[skip ci] ## [2.21.0](v2.20.7...v2.21.0) (2025-03-13) ### Features * remove http-assert ([#18](#18)) ([ce79a68](ce79a68))
Summary by CodeRabbit
ctx.assertfunction, improving clarity and maintainability.