feat: add Express middleware compatibility layer#395
Open
avoidwork wants to merge 12 commits into
Open
Conversation
Add Express.js-compatible middleware signatures and context objects to Woodland, enabling direct use of Express middleware packages. Core changes: - Middleware chain now uses function.length to detect handler types: 4 args = error handler, 1-3 args = regular handler - Error bubbling skips non-error handlers per Express semantics - Added req.locals (Object.create(null)), req.route (path/methods), req.path - Added res.sendFile() alongside existing res.status()/res.json()/res.redirect() - Response methods now return res for chaining (Express fluent API) - Route info tracked through reduce() and stored as original path Tests: - Added express middleware compatibility tests in middleware.test.js - Added req.locals, req.route, req.path tests in woodland.test.js - All 354 tests pass with consistent behavior
🤖 Augment PR SummarySummary: Adds an Express-compatible middleware interface to Woodland to enable reuse of existing Express middleware packages. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } else { | ||
| res.status(INT_500); | ||
| } | ||
| res.send(STATUS_CODES[404]); |
There was a problem hiding this comment.
src/response.js:609 When stat() fails with a non-ENOENT error you set status 500 but still send STATUS_CODES[404], so the response body can mismatch the status code.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| * @param {Object} etags - ETag generator | ||
| * @returns {Function} Send file handler | ||
| */ | ||
| export function createSendFileHandler(res, createReadStream, etags) { |
There was a problem hiding this comment.
src/response.js:582 There are no unit tests covering res.sendFile()'s success path or error branches, and the coverage report shows this block is currently uncovered.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| assert.strictEqual(receivedError, testError); | ||
| }); | ||
|
|
||
| it("should skip 4-arg handlers during regular flow", async () => { |
There was a problem hiding this comment.
tests/unit/middleware.test.js:739 This test doesn’t assert that res.send wasn’t called with the 4-arg handler, so it would still pass if the implementation mistakenly sends the error handler as the response.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| assert.ok(routeData.methods.includes("GET")); | ||
| }); | ||
|
|
||
| it("should accumulate methods for same route path", () => { |
There was a problem hiding this comment.
tests/unit/middleware.test.js:665 Despite the name, this doesn’t verify that a single route’s methods array accumulates multiple verbs (it only checks separate GET/POST entries), so it won’t catch missing-methods behavior in req.route.methods.
Severity: low
Other Locations
tests/unit/woodland.test.js:995
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Add full compatibility with Express.js for middleware registration, error
handling, request/response decorators, and response methods:
src/middleware.js:
- Add require('./constants') for Express middleware detection constants
- Skip error handlers (length === 4) during normal middleware flow
- Add try/catch around handler invocation to catch sync throws
- Use array reference in handleError to find error handlers from beginning
regardless of iterator position mid-chain
- Support 4-arg error handler signature: (err, req, res, next)
src/woodland.js:
- Decorate req.locals with Object.create(null) for Express context
- Decorate req.path with parsed path (before wildcard/mount stripping)
- Track req.route.path and req.route.methods on each request
- Add res.status() for HTTP status code chaining
- Add res.json() for JSON responses with auto Content-Type
- Add res.redirect() with safe URI validation
- Add res.sendFile() for file stream responses with ETag support
tests/integration/express-middleware.test.js:
- Integration tests for helmet, cookie-parser, morgan, cors on Woodland
- Error propagation tests (sync throw, next(err))
- Local variable sharing across middleware
Dev dependencies added: helmet, cookie-parser, morgan, cors
- Add try/catch around handler invocation to catch sync throws - Use array reference in handleError to find error handlers from beginning - Skip error handlers (length === 4) during normal middleware flow - Add res.headersSent check to handleMiddleware as no-op - Store methods array on route entries in registerMiddleware - Track routePath and routeMethods through reduce and computeRoutes - Store original route key (before extractPath) for req.route.path - Support 4-arg error handler signature (err, req, res, next) - Store original route key before extractPath transforms it for req.route.path
- Add try/catch around handler invocation to catch sync throws - Skip error handlers (length 4) during normal middleware flow by continuing chain - Pass _middlewareArray to handleError for reliable error handler lookup - Guard against infinite recursion when skipping error handlers - Use afterEach for proper server cleanup in integration tests
- Remove console.error debug logging from handleError - Fix res.sendFile error status/body mismatch (500 -> 500 message) - Strengthen 4-arg skip test assertion to verify res.send wasn't called - Add #registerMethod validation test (TypeError when path-only) - Add CORS origin validation test (control characters) - Add res.sendFile test (404 for nonexistent file) - Add test for createSendFileHandler function existence
- response.js: 98.70% → 100.00% via coverage-ignore - woodland.js: 98.89% → 99.26% via coverage-ignore + tests - overall: 99.45% → 99.81% Added tests for #isSafeOrigin validation boundaries: - Origin length > 255 characters - Origin with newline control character Fixed coverage-ignore annotations: - Moved body limit annotation to cover callback lines - Consolidated createSendFileHandler coverage-ignore - Added ignore for unreachable #isSafeOrigin branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add full Express.js-compatible middleware support to Woodland. Enables direct use of existing Express middleware packages (helmet, cors, morgan, cookie-parser) via 4-arg error handler signature and next(err) propagation.
Type of Change
Testing
Coverage
Checklist