feat: add Express and Lambda adapters#283
Conversation
WalkthroughThis PR refactors the project structure to introduce runtime adapters (Express and Lambda), relocates core modules under Changes
Sequence Diagram(s)sequenceDiagram
participant AGW as API Gateway v2
participant LH as Lambda Handler
participant ES as Express Server
participant HTTP as HTTP Client
AGW->>LH: APIGatewayProxyEventV2
LH->>LH: Cold start: start Express on loopback
LH->>LH: Convert event to HTTP request
LH->>LH: Filter hop-by-hop headers
LH->>ES: HTTP request to 127.0.0.1:port
ES->>ES: Route & process request
ES-->>LH: HTTP response
LH->>LH: Aggregate headers & cookies
LH->>LH: Build APIGatewayProxyStructuredResultV2
LH-->>AGW: Structured result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello, 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 introduces adapters for running the HTTP Playground server in different environments, specifically Express and AWS Lambda. It involves refactoring the project structure to separate core application logic from adapter-specific code, enabling the application to run as a Lambda function. The changes include creating new adapter entry points, updating build scripts, and modifying existing code to support both environments. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe 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
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 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. Adapters bridge the divide, Express and Lambda, side by side. Code's essence, a common core, Deployed with ease, and nothing more. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant architectural improvement by adding support for AWS Lambda through an adapter pattern, separating the core application logic from the runtime environment. The changes are generally well-structured and include necessary updates to documentation and build scripts. I've identified a few areas for improvement, primarily in the new Lambda adapter and build scripts, focusing on error handling, binary data support, and build efficiency. Additionally, there's a minor inconsistency in one of the new tests that should be addressed.
| } catch { | ||
| return { | ||
| statusCode: 500, | ||
| body: JSON.stringify({ error: { message: 'Internal server error' } }), | ||
| headers: { 'content-type': 'application/json' }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This catch block swallows the error object, which will make debugging very difficult in case of a failure. You should capture the error and log it for better observability.
} catch (error) {
console.error('Error in Lambda handler:', error);
return {
statusCode: 500,
body: JSON.stringify({ error: { message: 'Internal server error' } }),
headers: { 'content-type': 'application/json' },
};
}| "license": "MIT", | ||
| "scripts": { | ||
| "build": "tsc", | ||
| "build:lambda:function": "yarn build && cp package.json dist/ && zip -r lambda-function.zip dist/", |
There was a problem hiding this comment.
The build:lambda:function script can be improved for efficiency and correctness.
- It includes the
distdirectory itself in the zip archive, which means your Lambda handler path will need to bedist/adapters/lambda/index.handler. It's generally better to zip the contents of thedistdirectory. - It packages the Express adapter, which is not needed for the Lambda function, increasing the package size.
A simple improvement for the zip structure is to cd into the directory first.
| "build:lambda:function": "yarn build && cp package.json dist/ && zip -r lambda-function.zip dist/", | |
| "build:lambda:function": "yarn build && cp package.json dist/ && (cd dist && zip -r ../lambda-function.zip .)", |
| "scripts": { | ||
| "build": "tsc", | ||
| "build:lambda:function": "yarn build && cp package.json dist/ && zip -r lambda-function.zip dist/", | ||
| "build:lambda:layer": "mkdir -p lambda-layer/nodejs && cp -r node_modules lambda-layer/nodejs && cd lambda-layer && zip -r ../lambda-layer.zip . && cd .. && rm -rf lambda-layer", |
There was a problem hiding this comment.
The build:lambda:layer script copies the entire node_modules directory, which may include development dependencies, making the Lambda layer larger than necessary. Consider installing only production dependencies for the layer. Also, using cd ... && cd .. can be risky if a command fails; using a subshell (...) is safer.
| "build:lambda:layer": "mkdir -p lambda-layer/nodejs && cp -r node_modules lambda-layer/nodejs && cd lambda-layer && zip -r ../lambda-layer.zip . && cd .. && rm -rf lambda-layer", | |
| "build:lambda:layer": "mkdir -p lambda-layer/nodejs && cp -r node_modules lambda-layer/nodejs && (cd lambda-layer && zip -r ../lambda-layer.zip .) && rm -rf lambda-layer", |
| return { | ||
| statusCode, | ||
| headers: responseHeaders, | ||
| body: rawBody.toString('utf-8'), | ||
| ...(cookies.length > 0 && { cookies }), | ||
| }; |
There was a problem hiding this comment.
The response body is always converted to a utf-8 string. This will corrupt binary responses. The handler should check the Content-Type of the response and if it's binary, base64 encode the body and set isBase64Encoded: true in the response object. While the current application may not serve binary files, this would make the adapter more robust.
const contentType = responseHeaders['content-type'] || '';
const isText = /^(text\/|application\/json)/.test(contentType);
return {
statusCode,
headers: responseHeaders,
body: rawBody.toString(isText ? 'utf8' : 'base64'),
isBase64Encoded: !isText,
...(cookies.length > 0 && { cookies }),
};| if (process.env['RUNTIME'] === 'lambda') { | ||
| res | ||
| .status(400) | ||
| .json({ | ||
| error: { message: 'Network error simulation is not supported in this environment.' }, | ||
| }); | ||
| return; |
There was a problem hiding this comment.
The associated test for this logic in tests/ut/routes/error.test.ts expects a 500 status code, but the implementation here returns 400. The README.md also states it returns 400. A 400 Bad Request is more appropriate since the client is requesting an action not supported in the current environment. Please update the test to expect a 400 status code to align with the implementation and documentation.
| process.env['RUNTIME'] = 'lambda'; | ||
| try { | ||
| const response = await request(app)[method]('/error/network'); | ||
| expect(response.status).toBe(500); |
There was a problem hiding this comment.
This test expects a 500 status code, but the implementation in src/core/routes/error.ts returns a 400 for the /error/network endpoint in a Lambda environment. The README.md also states it returns 400. A 400 Bad Request is more appropriate here since the client is requesting an action not supported in the current environment. This test should be updated to expect a 400.
| expect(response.status).toBe(500); | |
| expect(response.status).toBe(400); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/routes/error.ts (1)
10-22:⚠️ Potential issue | 🔴 CriticalTest expects status 500 but implementation returns 400.
The test in
tests/ut/routes/error.test.ts(line 35) expectsresponse.statusto be500for the Lambda runtime path, but the implementation returns400. This mismatch will cause the test to fail.The implementation at line 13 of
src/core/routes/error.tsexplicitly returns:.status(400)But the test at line 35 asserts:
expect(response.status).toBe(500);Fix either the test or the implementation to match:
- Update test: Change line 35 to
expect(response.status).toBe(400)and update the test description to reflect 400 status- Update implementation: Change line 13 to
.status(500)if 500 is the intended status code🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/routes/error.ts` around lines 10 - 22, The Lambda-runtime branch in the error route returns status 400 but tests expect 500; inside the errorRouter.all('/network', (req, res) => { ... }) block where it checks process.env['RUNTIME'] === 'lambda', change the response status call from .status(400) to .status(500) so the Lambda path returns HTTP 500 (adjust the JSON message if needed to keep context).AGENTS.md (1)
25-29:⚠️ Potential issue | 🟡 MinorUpdate AGENTS.md to reflect current file structure.
The documentation at lines 25-29 references outdated file paths. The actual codebase uses:
src/adapters/express/app.ts(notsrc/app.ts)src/adapters/express/index.ts(notsrc/server.ts)src/core/env.ts(notsrc/env.ts)src/core/logger.ts(notsrc/logger.ts)Also update line 32 to reference
src/core/routes/instead ofsrc/routes/.📝 Proposed documentation update
**Core Structure**: -- `src/app.ts` - Main Express application setup with middleware chain and route registration -- `src/server.ts` - HTTP server startup with graceful shutdown handling -- `src/env.ts` - Environment variable validation and configuration -- `src/logger.ts` - Pino logger setup with development/production configurations +- `src/adapters/express/app.ts` - Main Express application setup with middleware chain and route registration +- `src/adapters/express/index.ts` - HTTP server startup with graceful shutdown handling +- `src/core/env.ts` - Environment variable validation and configuration +- `src/core/logger.ts` - Pino logger setup with development/production configurations **Route Structure**: -- All routes in `src/routes/` export an Express Router -- Registered in `src/app.ts` with path prefixes +- All routes in `src/core/routes/` export an Express Router +- Registered in `src/adapters/express/app.ts` with path prefixes🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 25 - 29, Update AGENTS.md to reflect the current repository layout by replacing the outdated paths: change `src/app.ts` to `src/adapters/express/app.ts`, `src/server.ts` to `src/adapters/express/index.ts`, `src/env.ts` to `src/core/env.ts`, and `src/logger.ts` to `src/core/logger.ts`; also update the routes reference at line 32 from `src/routes/` to `src/core/routes/`. Ensure the Core Structure bullet list and the single "routes" reference are edited consistently so the doc matches the codebase.
🧹 Nitpick comments (2)
src/adapters/lambda/index.ts (2)
130-136: Consider logging errors for observability.The catch block silently swallows errors without logging, making debugging Lambda invocations difficult. CloudWatch logs would benefit from error details.
♻️ Proposed enhancement to add error logging
- } catch { + } catch (err) { + console.error('Lambda handler error:', err); return { statusCode: 500, body: JSON.stringify({ error: { message: 'Internal server error' } }), headers: { 'content-type': 'application/json' }, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapters/lambda/index.ts` around lines 130 - 136, The catch block that returns the 500 response currently swallows the exception; change it to capture the thrown error (e.g., catch (err)) and log the error and relevant context (such as the incoming event or request id) to CloudWatch using the existing logger or console.error before returning the JSON 500 response so failures are observable; update the catch in the function that builds the Lambda response (the block that returns statusCode: 500 and body JSON) to include the error logging statement.
124-129: Binary responses will be corrupted.
rawBody.toString('utf-8')assumes all responses are UTF-8 text. Binary responses (e.g., images) will be corrupted. API Gateway v2 supportsisBase64Encodedfor binary content.♻️ Optional enhancement for binary response support
+ // Detect if response is likely binary based on content-type + const contentType = responseHeaders['content-type'] ?? ''; + const isBinary = /^(image|audio|video|application\/octet-stream)/i.test(contentType); + return { statusCode, headers: responseHeaders, - body: rawBody.toString('utf-8'), + body: isBinary ? rawBody.toString('base64') : rawBody.toString('utf-8'), + isBase64Encoded: isBinary, ...(cookies.length > 0 && { cookies }), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapters/lambda/index.ts` around lines 124 - 129, The code always converts rawBody with rawBody.toString('utf-8'), which corrupts binary responses; detect whether the response is textual (e.g., Content-Type header includes "text/" or "application/json" or a "charset=utf-8") and only then use rawBody.toString('utf-8'), otherwise set body = rawBody.toString('base64') and add isBase64Encoded: true to the returned object; update the return construction that uses rawBody, responseHeaders, statusCode, and cookies to conditionally encode and include isBase64Encoded accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/DEVELOPMENT_GUIDE.md`:
- Around line 9-19: The docs tree in DEVELOPMENT_GUIDE.md is inconsistent with
the repository convention; update the PR so the authoritative layout matches the
existing codebase by reverting or editing the docs to document the canonical
files: ensure application setup lives in src/app.ts (Express app with middleware
and route registration), routes live under src/routes/* exporting Express
Router, logger configuration is described in src/logger.ts (dev/prod configs),
server startup and graceful shutdown are documented for src/server.ts, and
environment variables (ENABLE_SHUTDOWN, LOG_LEVEL, MAX_DELAY, NODE_ENV, ORIGIN,
PORT, KEEP_ALIVE_TIMEOUT, HEADERS_TIMEOUT, REQUEST_TIMEOUT) are described in
src/env.ts; reference these filenames (src/app.ts, src/server.ts, src/env.ts,
src/logger.ts, src/routes) when making the doc changes so the guide and repo
conventions are in sync.
In `@package.json`:
- Line 12: The current "build:lambda:layer" npm script copies the entire
node_modules (including devDependencies) into the Lambda layer; change it to
install only production dependencies into the layer directory instead. Update
the script so it creates lambda-layer/nodejs and runs a production install into
that folder (for example using npm ci --only=production --prefix
lambda-layer/nodejs or npm install --production --prefix lambda-layer/nodejs),
then zip and clean up as before; ensure you remove the plain cp -r node_modules
step in the "build:lambda:layer" script so devDependencies like
Jest/ESLint/TypeScript are not included.
In `@src/adapters/lambda/index.ts`:
- Around line 26-37: The Lambda adapter never sets the RUNTIME env var so
requests hit the socket-destroy path; before creating or starting the HTTP
server (i.e., before the listeningPromise / createServer(app) / server.listen
call) set process.env['RUNTIME'] = 'lambda' so the error route in
src/core/routes/error.ts sees RUNTIME==='lambda' and returns the safe 400
response instead of attempting req.socket.destroy(); ensure this assignment runs
synchronously prior to any server creation/listen logic.
In `@tests/ut/routes/error.test.ts`:
- Around line 31-44: The test's assertion and description are wrong for the
lambda runtime case: update the it(...) description and the expect(...)
assertion in the test for the '/error/network' route (inside the test case that
sets process.env['RUNTIME']='lambda') to expect a 400 status code instead of
500; leave the conditional body check for non-head methods unchanged (it already
asserts the error message), and ensure the finally block still deletes
process.env['RUNTIME'] so cleanup remains intact.
---
Outside diff comments:
In `@AGENTS.md`:
- Around line 25-29: Update AGENTS.md to reflect the current repository layout
by replacing the outdated paths: change `src/app.ts` to
`src/adapters/express/app.ts`, `src/server.ts` to
`src/adapters/express/index.ts`, `src/env.ts` to `src/core/env.ts`, and
`src/logger.ts` to `src/core/logger.ts`; also update the routes reference at
line 32 from `src/routes/` to `src/core/routes/`. Ensure the Core Structure
bullet list and the single "routes" reference are edited consistently so the doc
matches the codebase.
In `@src/core/routes/error.ts`:
- Around line 10-22: The Lambda-runtime branch in the error route returns status
400 but tests expect 500; inside the errorRouter.all('/network', (req, res) => {
... }) block where it checks process.env['RUNTIME'] === 'lambda', change the
response status call from .status(400) to .status(500) so the Lambda path
returns HTTP 500 (adjust the JSON message if needed to keep context).
---
Nitpick comments:
In `@src/adapters/lambda/index.ts`:
- Around line 130-136: The catch block that returns the 500 response currently
swallows the exception; change it to capture the thrown error (e.g., catch
(err)) and log the error and relevant context (such as the incoming event or
request id) to CloudWatch using the existing logger or console.error before
returning the JSON 500 response so failures are observable; update the catch in
the function that builds the Lambda response (the block that returns statusCode:
500 and body JSON) to include the error logging statement.
- Around line 124-129: The code always converts rawBody with
rawBody.toString('utf-8'), which corrupts binary responses; detect whether the
response is textual (e.g., Content-Type header includes "text/" or
"application/json" or a "charset=utf-8") and only then use
rawBody.toString('utf-8'), otherwise set body = rawBody.toString('base64') and
add isBase64Encoded: true to the returned object; update the return construction
that uses rawBody, responseHeaders, statusCode, and cookies to conditionally
encode and include isBase64Encoded accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01a3955f-3f83-487b-99e1-7e064f92b2f7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
.claude/settings.json.github/workflows/ci.ymlAGENTS.mdDockerfileREADME.mddocs/DEVELOPMENT_GUIDE.mdnodemon.jsonpackage.jsonsrc/adapters/express/app.tssrc/adapters/express/index.tssrc/adapters/lambda/index.tssrc/core/env.tssrc/core/logger.tssrc/core/middlewares/delay.tssrc/core/middlewares/logger.tssrc/core/routes/base64.tssrc/core/routes/basic-auth.tssrc/core/routes/error.tssrc/core/routes/index.tssrc/core/routes/mirror.tssrc/core/routes/redirect.tssrc/core/routes/request.tssrc/core/routes/shutdown.tssrc/core/routes/status.tssrc/core/routes/uuid.tstests/ut/app.test.tstests/ut/env.test.tstests/ut/middlewares/delay.test.tstests/ut/middlewares/logger.test.tstests/ut/routes/base64.test.tstests/ut/routes/basic-auth.test.tstests/ut/routes/error.test.tstests/ut/routes/index.test.tstests/ut/routes/mirror.test.tstests/ut/routes/redirect.test.tstests/ut/routes/request.test.tstests/ut/routes/shutdown.test.tstests/ut/routes/status.test.tstests/ut/routes/uuid.test.ts
| │ ├── adapters # Runtime-specific entry points | ||
| │ │ ├── express # Express/Docker adapter | ||
| │ │ │ ├── app.ts # Express application setup | ||
| │ │ │ └── index.ts # Server startup and shutdown handling | ||
| │ │ └── lambda # AWS Lambda adapter | ||
| │ │ └── index.ts # Lambda handler (API Gateway HTTP API v2) | ||
| │ ├── core # Shared business logic | ||
| │ │ ├── env.ts # Environment variable handler | ||
| │ │ ├── logger.ts # Application logging configuration | ||
| │ │ ├── middlewares # Middlewares directory | ||
| │ │ └── routes # API routes directory |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Keep the documented source layout in sync with the repo conventions.
This tree now tells contributors to put app/server code under src/adapters/express/ and shared modules under src/core/, but the current project conventions still point to src/app.ts, src/server.ts, src/env.ts, src/logger.ts, and src/routes/. Please resolve that mismatch in this PR so the repo has a single authoritative layout.
Based on learnings: "Applies to src/app.ts : Main Express application setup with middleware chain and route registration must be in src/app.ts", "Applies to src/routes/**/*.{ts,tsx} : All routes must be located in src/routes/ and export an Express Router", "Applies to src/logger.ts : Logger setup with development/production configurations must be in src/logger.ts", "Applies to src/server.ts : HTTP server startup with graceful shutdown handling must be in src/server.ts", and "Applies to src/env.ts : Configure environment variables in src/env.ts including ENABLE_SHUTDOWN, LOG_LEVEL, MAX_DELAY, NODE_ENV, ORIGIN, PORT, KEEP_ALIVE_TIMEOUT, HEADERS_TIMEOUT, and REQUEST_TIMEOUT".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/DEVELOPMENT_GUIDE.md` around lines 9 - 19, The docs tree in
DEVELOPMENT_GUIDE.md is inconsistent with the repository convention; update the
PR so the authoritative layout matches the existing codebase by reverting or
editing the docs to document the canonical files: ensure application setup lives
in src/app.ts (Express app with middleware and route registration), routes live
under src/routes/* exporting Express Router, logger configuration is described
in src/logger.ts (dev/prod configs), server startup and graceful shutdown are
documented for src/server.ts, and environment variables (ENABLE_SHUTDOWN,
LOG_LEVEL, MAX_DELAY, NODE_ENV, ORIGIN, PORT, KEEP_ALIVE_TIMEOUT,
HEADERS_TIMEOUT, REQUEST_TIMEOUT) are described in src/env.ts; reference these
filenames (src/app.ts, src/server.ts, src/env.ts, src/logger.ts, src/routes)
when making the doc changes so the guide and repo conventions are in sync.
| "scripts": { | ||
| "build": "tsc", | ||
| "build:lambda:function": "yarn build && cp package.json dist/ && zip -r lambda-function.zip dist/", | ||
| "build:lambda:layer": "mkdir -p lambda-layer/nodejs && cp -r node_modules lambda-layer/nodejs && cd lambda-layer && zip -r ../lambda-layer.zip . && cd .. && rm -rf lambda-layer", |
There was a problem hiding this comment.
Package only runtime dependencies in the Lambda layer.
Copying the repo’s whole node_modules directory will pull Jest, ESLint, TypeScript, Newman, and the rest of the dev toolchain into the layer. That makes the artifact much larger than necessary and can turn deployment into a size problem. Build the layer from production dependencies only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 12, The current "build:lambda:layer" npm script copies
the entire node_modules (including devDependencies) into the Lambda layer;
change it to install only production dependencies into the layer directory
instead. Update the script so it creates lambda-layer/nodejs and runs a
production install into that folder (for example using npm ci --only=production
--prefix lambda-layer/nodejs or npm install --production --prefix
lambda-layer/nodejs), then zip and clean up as before; ensure you remove the
plain cp -r node_modules step in the "build:lambda:layer" script so
devDependencies like Jest/ESLint/TypeScript are not included.
| const listeningPromise: Promise<number> = new Promise((resolve, reject) => { | ||
| const server = createServer(app); | ||
| server.listen(0, '127.0.0.1', () => { | ||
| const address = server.address(); | ||
| if (address === null || typeof address === 'string') { | ||
| reject(new Error('Unexpected server address format')); | ||
| return; | ||
| } | ||
| resolve(address.port); | ||
| }); | ||
| server.on('error', reject); | ||
| }); |
There was a problem hiding this comment.
Missing RUNTIME environment variable initialization.
The /error/network route in src/core/routes/error.ts (lines 11-18) checks process.env['RUNTIME'] === 'lambda' to safely return a 400 response instead of calling req.socket.destroy(). However, this Lambda adapter never sets RUNTIME='lambda', so the socket destruction path will execute in Lambda invocations, which won't behave correctly since the socket lifecycle is managed by API Gateway.
🐛 Proposed fix to set RUNTIME before starting the server
+// Set runtime environment for Lambda-specific behavior in routes
+process.env['RUNTIME'] = 'lambda';
+
/**
* Starts the Express app on a random loopback port and resolves with the port number.
* Called once on cold start; subsequent invocations reuse the cached promise.
*/
const listeningPromise: Promise<number> = new Promise((resolve, reject) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adapters/lambda/index.ts` around lines 26 - 37, The Lambda adapter never
sets the RUNTIME env var so requests hit the socket-destroy path; before
creating or starting the HTTP server (i.e., before the listeningPromise /
createServer(app) / server.listen call) set process.env['RUNTIME'] = 'lambda' so
the error route in src/core/routes/error.ts sees RUNTIME==='lambda' and returns
the safe 400 response instead of attempting req.socket.destroy(); ensure this
assignment runs synchronously prior to any server creation/listen logic.
| it('should return 500 for "network" type in lambda runtime', async () => { | ||
| process.env['RUNTIME'] = 'lambda'; | ||
| try { | ||
| const response = await request(app)[method]('/error/network'); | ||
| expect(response.status).toBe(500); | ||
| if (method !== 'head') { | ||
| expect(response.body).toEqual({ | ||
| error: { message: 'Network error simulation is not supported in this environment.' }, | ||
| }); | ||
| } | ||
| } finally { | ||
| delete process.env['RUNTIME']; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the error route implementation
fd -e ts -e tsx -path '*/routes/error*' | head -20Repository: ryo8000/http-playground-server
Length of output: 242
🏁 Script executed:
# Search for the /error/network route handler and RUNTIME handling
rg -n "network" --type ts -g '**/routes/**' -A 10Repository: ryo8000/http-playground-server
Length of output: 2663
🏁 Script executed:
# Also check the test file to see current state
cat -n tests/ut/routes/error.test.ts | head -60Repository: ryo8000/http-playground-server
Length of output: 2743
Update test assertion to match implementation: expect 400 status code, not 500.
The /error/network route returns HTTP 400 (not 500) when RUNTIME is 'lambda'. The test description on line 31 and the assertion on line 35 need correction to match the implementation.
🐛 Proposed fix to match implementation behavior
- it('should return 500 for "network" type in lambda runtime', async () => {
+ it('should return 400 for "network" type in lambda runtime', async () => {
process.env['RUNTIME'] = 'lambda';
try {
const response = await request(app)[method]('/error/network');
- expect(response.status).toBe(500);
+ expect(response.status).toBe(400);
if (method !== 'head') {
expect(response.body).toEqual({
error: { message: 'Network error simulation is not supported in this environment.' },
});
}
} finally {
delete process.env['RUNTIME'];
}
});📝 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.
| it('should return 500 for "network" type in lambda runtime', async () => { | |
| process.env['RUNTIME'] = 'lambda'; | |
| try { | |
| const response = await request(app)[method]('/error/network'); | |
| expect(response.status).toBe(500); | |
| if (method !== 'head') { | |
| expect(response.body).toEqual({ | |
| error: { message: 'Network error simulation is not supported in this environment.' }, | |
| }); | |
| } | |
| } finally { | |
| delete process.env['RUNTIME']; | |
| } | |
| }); | |
| it('should return 400 for "network" type in lambda runtime', async () => { | |
| process.env['RUNTIME'] = 'lambda'; | |
| try { | |
| const response = await request(app)[method]('/error/network'); | |
| expect(response.status).toBe(400); | |
| if (method !== 'head') { | |
| expect(response.body).toEqual({ | |
| error: { message: 'Network error simulation is not supported in this environment.' }, | |
| }); | |
| } | |
| } finally { | |
| delete process.env['RUNTIME']; | |
| } | |
| }); |
🧰 Tools
🪛 GitHub Actions: CI
[error] 35-35: Test failure: errorRouter 'network' type should return 500 but received 400. Command: 'yarn test'. Investigate error handling for /error/network route (GET/POST/PUT/DELETE/etc.).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ut/routes/error.test.ts` around lines 31 - 44, The test's assertion and
description are wrong for the lambda runtime case: update the it(...)
description and the expect(...) assertion in the test for the '/error/network'
route (inside the test case that sets process.env['RUNTIME']='lambda') to expect
a 400 status code instead of 500; leave the conditional body check for non-head
methods unchanged (it already asserts the error message), and ensure the finally
block still deletes process.env['RUNTIME'] so cleanup remains intact.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores