fix: make ctx.assert errors instanceof HttpError - #1998
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughKoa now uses structural Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue
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 |
Reviewer's GuideUpdates Koa's HTTP error identity semantics so ctx.assert failures from nested http-errors are recognized as Koa.HttpError instances while preserving the original error type, assertion behavior, and metadata; adds comprehensive regression tests. Sequence diagram for ctx.assert HTTP error identitysequenceDiagram
participant Caller
participant Context
participant HttpAssert
participant HttpError
Caller->>Context: assert(condition, status, message, options)
Context->>HttpAssert: assert(condition, status, message, options)
HttpAssert-->>Context: nested HttpError
Context->>HttpError: HttpError[Symbol.hasInstance](error)
HttpError-->>Caller: true for HTTP-error-like error
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1998 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 9 9
Lines 2110 2125 +15
=======================================
+ Hits 2108 2123 +15
Misses 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0be5fa7 to
94e60bd
Compare
|
cr plz @jonathanong @fengmk2 @3imed-jaberi |
af0153a to
3161670
Compare
|
@sourcery-ai re-review |
|
Hey @yowainwright, I've posted a new review for you! |
befc3bf to
7d41da9
Compare
7d41da9 to
a21ca97
Compare
Human Summary
ctx.assert()errors passKoa.isHttpError(err)but failerr instanceof Koa.HttpErrorbecause they come fromhttp-assert's nestedhttp-errors.This wraps
http-assertfailures with Koa'shttp-errorswhile keeping the existing assertion behavior.Approach comparisons
I took 2 routes and landed on the last version b/c of lack of risk and readability
http-errorsHttpError; leavectx.assertashttp-asserthttp-assertbehavior exactlyctx.assert()errors failerr instanceof Koa.HttpErrorwhenhttp-assertuses a nestedhttp-errorscopyctx.assertdelegates tohttp-assert; Koa exports rootcreateHttpError.HttpErrorctx.assert; rebuild failures with roothttp-errorsctx.assertfailures passinstanceof Koa.HttpError; status-specific Koa constructors can matchObject.assign({}, err)copies only enumerable own properties; stack, descriptors, symbols, and identity can changehttp-assertthrowscreateError(status, msg, opts);http-errorscreates or mutates error fields based on its own status parsingctx.assert; add duck-typedSymbol.hasInstancetoKoa.HttpErrorctx.assert()errors; keeps original constructor, stack, own props, and nestedhttp-errorsinstanceof; fixeserr instanceof Koa.HttpErrorKoa.HttpError instanceofsemantics from prototype-only to HTTP-error-shaped values; does not make errors instances of Koa's status-specific constructorshttp-errors.isHttpErrorduck-typing fallback:Error, booleanexpose, numericstatusCode, and matchingstatusSummary by Sourcery
Make
ctx.assert()failures compatible with Koa’s HTTP error type checks.Bug Fixes:
ctx.assert()and its named assertion methods recognize asKoa.HttpErrorinstances while preserving their existing status, messages, and custom properties.Enhancements:
Summary by CodeRabbit
Bug Fixes
Tests