feat(file-upload): add AWS S3 and Cloudflare R2 upload#143
Conversation
providers
Add two new variants to the file-upload component registry:
- AWS S3: uses @aws-sdk/client-s3 with
PutObjectCommand/DeleteObjectCommand,
generates a public URL in the format
https://<bucket>.s3.<region>.amazonaws.com/<key>.
Requires: AWS_REGION, AWS_ACCESS_KEY_ID,
AWS_SECRET_ACCESS_KEY, AWS_S3_BUCKET_NAME.
- Cloudflare R2: S3-compatible via @aws-sdk/client-s3 pointed
at the R2 endpoint
(https://<account-id>.r2.cloudflarestorage.com). Public URL
is built from a
configurable R2_PUBLIC_URL env var.
Requires: R2_ACCOUNT_ID, R2_ACCESS_KEY_ID,
R2_SECRET_ACCESS_KEY, R2_BUCKET_NAME, R2_PUBLIC_URL.
Both providers scaffold multer (memoryStorage) → SDK upload
pipeline with
single-file, multi-file, and delete endpoints. Templates are
provided for
both MVC and Feature architectures. Env keys are
auto-injected into .env
and .env.example on install.
|
@pawanbhayde is attempting to deploy a commit to the akkaldhami's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (59)
📝 WalkthroughWalkthroughThe PR adds pre-commit enforcement to block main/master commits, updates the file-upload component registry with AWS S3 and Cloudflare R2 provider variants, and introduces complete Express application templates for both storage backends in feature and mvc patterns. Each variant includes environment validation, controllers, routing, service layers, and shared utilities. ChangesFile-upload components for Express
Sequence Diagram(s)sequenceDiagram
participant Client as Express Client
participant Routes as Router
participant Controller as Controller Handler
participant Service as Upload Service
participant Storage as Cloud Storage
Client->>Routes: POST /file (multipart)
Routes->>Controller: Multer validates MIME type and size
Controller->>Service: uploadToS3/uploadToR2(file)
Service->>Storage: PutObjectCommand
Storage-->>Service: Object stored with metadata
Service-->>Controller: UploadResult {url, key, size}
Controller-->>Client: 201 ApiResponse with result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Adds two new file-upload provider variants (AWS S3 and Cloudflare R2) to the Express template registry, including MVC and Feature-architecture scaffolds that implement single-file upload, multi-file upload, and delete endpoints backed by S3-compatible APIs.
Changes:
- Added AWS S3 provider templates (MVC + Feature) using
@aws-sdk/client-s3for put/delete operations and constructing public S3 URLs. - Added Cloudflare R2 provider templates (MVC + Feature) using an S3-compatible
S3Clientendpoint and constructing public URLs fromR2_PUBLIC_URL. - Registered both providers in
packages/registry/component/file-upload.jsonwith dependencies and env-key injection.
Reviewed changes
Copilot reviewed 55 out of 56 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/logger.ts | Adds pino logger configuration for the R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/async-handler.ts | Adds async route wrapper helper for the R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/api-response.ts | Adds standardized API response helper for the R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/api-error.ts | Adds typed API error helper for the R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/r2.service.ts | Implements R2 upload/delete operations via S3-compatible SDK. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/routes/upload.routes.ts | Defines upload/delete routes for R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.ts | Adds multer memory storage + file type/size limits for R2 MVC. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/error-handler.ts | Adds global error handler middleware for R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/controllers/upload.controller.ts | Adds controllers for single/multi upload and delete (R2 MVC). |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/constants/status-codes.ts | Adds HTTP status code constants/types for R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/configs/r2.ts | Configures S3Client for Cloudflare R2 (MVC). |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/configs/env.ts | Adds zod-based env validation for R2 MVC template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/app.upload-file.test.ts | Adds R2 MVC express app bootstrap wiring routes and middleware. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/utils/logger.ts | Adds pino logger configuration for the R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/utils/async-handler.ts | Adds async route wrapper helper for the R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/utils/api-response.ts | Adds standardized API response helper for the R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/upload-file.ts | Adds multer memory storage + file type/size limits for R2 Feature. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/error-handler.ts | Adds global error handler middleware for R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/errors/api-error.ts | Adds typed API error helper for R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/constants/status-codes.ts | Adds HTTP status code constants/types for R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/configs/r2.ts | Configures S3Client for Cloudflare R2 (Feature). |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/configs/env.ts | Adds zod-based env validation for R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/routes/index.ts | Adds top-level routing composition for R2 Feature template. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.service.ts | Implements R2 upload/delete operations (Feature module). |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.routes.ts | Defines upload/delete routes for R2 Feature module. |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.controller.ts | Adds controllers for single/multi upload and delete (R2 Feature). |
| packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/app.upload-file.test.ts | Adds R2 Feature express app bootstrap wiring routes and middleware. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/logger.ts | Adds pino logger configuration for the S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/async-handler.ts | Adds async route wrapper helper for the S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-response.ts | Adds standardized API response helper for the S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-error.ts | Adds typed API error helper for the S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/services/s3.service.ts | Implements S3 upload/delete operations via AWS SDK. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/routes/upload.routes.ts | Defines upload/delete routes for S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/middlewares/upload-file.ts | Adds multer memory storage + file type/size limits for S3 MVC. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/middlewares/error-handler.ts | Adds global error handler middleware for S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/controllers/upload.controller.ts | Adds controllers for single/multi upload and delete (S3 MVC). |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/constants/status-codes.ts | Adds HTTP status code constants/types for S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/configs/s3.ts | Configures AWS S3Client (MVC). |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/configs/env.ts | Adds zod-based env validation for S3 MVC template. |
| packages/templates/node/express/component/file-upload/aws-s3/mvc/src/app.upload-file.test.ts | Adds S3 MVC express app bootstrap wiring routes and middleware. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/utils/logger.ts | Adds pino logger configuration for the S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/utils/async-handler.ts | Adds async route wrapper helper for the S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/utils/api-response.ts | Adds standardized API response helper for the S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/upload-file.ts | Adds multer memory storage + file type/size limits for S3 Feature. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/error-handler.ts | Adds global error handler middleware for S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/errors/api-error.ts | Adds typed API error helper for S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/constants/status-codes.ts | Adds HTTP status code constants/types for S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/s3.ts | Configures AWS S3Client (Feature). |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/env.ts | Adds zod-based env validation for S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/routes/index.ts | Adds top-level routing composition for S3 Feature template. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.service.ts | Implements S3 upload/delete operations (Feature module). |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.routes.ts | Defines upload/delete routes for S3 Feature module. |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.controller.ts | Adds controllers for single/multi upload and delete (S3 Feature). |
| packages/templates/node/express/component/file-upload/aws-s3/feature/src/app.upload-file.test.ts | Adds S3 Feature express app bootstrap wiring routes and middleware. |
| packages/registry/component/file-upload.json | Registers AWS S3 and Cloudflare R2 variants, deps, and env keys. |
| .husky/pre-commit | Adjusts formatting/whitespace in the pre-commit hook script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const ext = file.originalname.split(".").pop(); | ||
| const key = `${folder}/${randomUUID()}.${ext}`; | ||
|
|
| const ext = file.originalname.split(".").pop(); | ||
| const key = `${folder}/${randomUUID()}.${ext}`; | ||
|
|
| const ext = file.originalname.split(".").pop(); | ||
| const key = `${folder}/${randomUUID()}.${ext}`; | ||
|
|
| const url = `${env.R2_PUBLIC_URL}/${key}`; | ||
|
|
||
| return { url, key, size: file.size }; |
| const url = `${env.R2_PUBLIC_URL}/${key}`; | ||
|
|
||
| return { url, key, size: file.size }; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (7)
packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.controller.ts (1)
21-21: ⚡ Quick winUnsafe type assertion on
req.files.The type assertion
req.files as Express.Multer.File[]is only safe when usingupload.array()orupload.single(). Since the route does useupload.array("files", 10), this is currently safe, but the code is brittle to route middleware changes.Add a runtime check:
🛡️ Add runtime validation
- const files = req.files as Express.Multer.File[]; + const files = req.files as Express.Multer.File[] | undefined; - if (!files || files.length === 0) { + if (!files || !Array.isArray(files) || files.length === 0) { return next(ApiError.badRequest("Files are required")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.controller.ts` at line 21, The code unsafely asserts req.files as Express.Multer.File[]; replace this brittle assertion with a runtime check: verify Array.isArray(req.files) (and optionally that items have expected properties like originalname/mimetype/buffer) before treating them as files, and return a 400/error if the check fails; update the handler that uses upload.array("files", 10) to narrow the type only after the runtime validation (e.g., assign to files after the guard) so changes to middleware won’t cause a runtime crash.packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.service.ts (1)
47-57: 💤 Low valueUnused function
deleteManyFromR2.The
deleteManyFromR2function is defined but never used in the controllers. Consider removing it or exposing a bulk-delete endpoint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.service.ts` around lines 47 - 57, The function deleteManyFromR2 is defined but unused; either remove it or wire it up as a bulk-delete API: if you want to keep it, add a controller method (e.g., UploadController.deleteMany or a new handler in upload.controller.ts) that validates req.body.keys is a non-empty string[] and calls deleteManyFromR2(keys), then register that handler on the upload router (e.g., POST /upload/delete-many) and return an appropriate status (204 on success or 400/500 on error). If you prefer removal, delete deleteManyFromR2 from upload.service.ts and any unused imports to avoid dead code. Ensure you reference deleteManyFromR2 when implementing or removing to keep changes targeted.packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/upload-file.ts (1)
17-21: ⚡ Quick winReturn an explicit client error for unsupported MIME types.
On Line 19,
cb(null, false)silently drops invalid files, which typically surfaces as a generic “no file” response instead of a clear validation failure. Prefer returning anApiError.badRequest(...)so clients get a deterministic 400.♻️ Proposed fix
import multer from "multer"; +import { ApiError } from "../errors/api-error"; @@ const fileFilter: multer.Options["fileFilter"] = (_req, file, cb) => { if (!ALLOWED_FILE_TYPES.includes(file.mimetype)) { - return cb(null, false); + return cb(ApiError.badRequest(`Unsupported file type: ${file.mimetype}`)); } cb(null, true); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/upload-file.ts` around lines 17 - 21, The fileFilter currently calls cb(null, false) for unsupported MIME types which silently drops the file; replace that with calling the multer callback with an ApiError bad request so the client receives a 400. Update the fileFilter (multer.Options["fileFilter"]) to call cb(new ApiError.badRequest(`Unsupported file type: ${file.mimetype}`)) for types not in ALLOWED_FILE_TYPES, and ensure ApiError is imported/available in this module so the error propagates as a 400 instead of a silent “no file.”packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.routes.ts (1)
13-13: ⚡ Quick winPrefer route/query identifier over DELETE body payload.
Line 13 uses a body-based key for deletion. Using a route/query identifier is typically more interoperable across clients/proxies for DELETE requests.
Suggested route shape
-router.delete("/", deleteFile); +router.delete("/:key", deleteFile);(Then read
req.params.keyin the controller.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.routes.ts` at line 13, The DELETE route currently uses a body payload (router.delete("/", deleteFile)); change the route to accept the file identifier as a path param (e.g., router.delete("/:key", deleteFile)) and update the deleteFile controller to read the identifier from req.params.key instead of req.body.key; also update any callers/tests/clients to call DELETE /:key (or use query param) and ensure validation/error handling still applies for missing/invalid key in the deleteFile handler.packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/upload-file.ts (1)
17-21: ⚡ Quick winFile validation error handling is already explicit; consider moving MIME type rejection into the fileFilter for consistency.
The current code checks
if (!req.file)in the controller and returns an explicit error, which produces the correct 400 response. However, you can achieve earlier validation by passing an error to the multer callback directly. SinceApiErrorextendsError,cb(ApiError.badRequest(...))is compatible with multer's fileFilter signature and will be caught by your error-handling middleware.This is optional — the current approach works correctly — but centralizing validation logic in the fileFilter is a minor architectural improvement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/upload-file.ts` around lines 17 - 21, Move MIME-type rejection into the multer fileFilter by passing an ApiError to the multer callback instead of silently returning false; in the fileFilter function (named fileFilter) replace the current cb(null, false) branch with cb(ApiError.badRequest("...invalid file type...")) so multer forwards the error to your error-handling middleware, and then remove or simplify the controller's explicit if (!req.file) check since validation is handled earlier.packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-response.ts (1)
47-47: ⚡ Quick winUse
STATUS_CODES.OKinstead of literal200for consistency.The default value uses a magic number
200, while theokandcreatedmethods use named constants fromSTATUS_CODES. Using the named constant improves maintainability and consistency throughout the codebase.♻️ Proposed fix
static Success<T>( res: Response, message: string, data?: T, - statusCode: StatusCode = 200 + statusCode: StatusCode = STATUS_CODES.OK ): Response {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-response.ts` at line 47, Replace the magic number default for the statusCode property with the named constant used elsewhere: change the default in the ApiResponse (the declaration where statusCode: StatusCode = 200) to use STATUS_CODES.OK so it matches the ok/created helpers and improves consistency.packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/async-handler.ts (1)
9-15: ⚡ Quick winAdd explicit return type annotation.
The
AsyncHandlerfunction should declare its return type explicitly for better type safety and API clarity.♻️ Proposed fix
-export function AsyncHandler(fn: AsyncRouteHandler) { +export function AsyncHandler( + fn: AsyncRouteHandler +): (req: Request, res: Response, next: NextFunction) => void { return function (req: Request, res: Response, next: NextFunction) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/async-handler.ts` around lines 9 - 15, The AsyncHandler function lacks an explicit return type; update the AsyncHandler signature to declare its return type explicitly (e.g., a function type that takes Request, Response, NextFunction and returns void or an Express RequestHandler) so the exported symbol has clear typing; locate the AsyncHandler function and annotate its return type using the existing AsyncRouteHandler, Request, Response, and NextFunction types to ensure callers and TypeScript inferencing see the correct API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/app.upload-file.test.ts`:
- Around line 7-20: The module currently calls app.listen(PORT) at import time
(using app, Routes, errorHandler, env.PORT, logger), causing side effects;
refactor by removing the app.listen(...) call from this file, export the
configured Express application (export default app), and move the server startup
into a new bootstrap entrypoint (e.g., server.ts) that imports the exported app,
reads env.PORT, and calls app.listen with the logger call; ensure Routes and
errorHandler remain wired to the exported app.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.controller.ts`:
- Around line 37-43: The controller currently only checks presence of
req.body.key before calling deleteFromS3; update the logic to validate that key
is a string and that it begins with a safe upload prefix (e.g. "uploads/files/")
before calling deleteFromS3, returning next(ApiError.badRequest(...)) for
non-string or disallowed-prefix values; use a named constant (e.g.
UPLOAD_PREFIX) or config value to check key.startsWith(UPLOAD_PREFIX) so keys
outside the upload namespace are rejected.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.service.ts`:
- Around line 47-56: The deleteManyFromS3 helper currently always calls
DeleteObjectsCommand and ignores its response, which can hide per-object
failures and also unnecessarily calls S3 when keys is empty; update
deleteManyFromS3 to short-circuit and return immediately when keys.length === 0,
then await the DeleteObjectsCommand response, inspect the returned Errors array
(response.Errors) and handle any entries (e.g., throw a descriptive error or log
and rethrow) so partial failures are surfaced; reference s3Client,
DeleteObjectsCommand and deleteManyFromS3 when implementing this change.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/env.ts`:
- Around line 15-18: The env schema currently allows empty AWS values because
AWS_REGION, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_S3_BUCKET_NAME are
defined as z.string() which accepts "" — update those schema entries to require
non-empty strings by using z.string().min(1) for each of AWS_REGION,
AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and AWS_S3_BUCKET_NAME in the env.ts
config so boot-time validation rejects empty values.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/error-handler.ts`:
- Around line 15-23: The middleware currently treats non-ApiError errors as 500;
update the error-handler (the function that inspects err in error-handler.ts) to
special-case multer.MulterError: if err is an instance of multer.MulterError,
set statusCode to 413 and a clear message when err.code === 'LIMIT_FILE_SIZE'
(file too large), otherwise set statusCode to 400 for other multer validation
failures and include err.message/details in the errors payload; preserve
existing ApiError handling (err instanceof ApiError) and keep default 500 for
truly unexpected errors.
In
`@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/configs/env.ts`:
- Around line 5-7: The NODE_ENV schema currently defaults to "development" which
is unsafe; update the z.enum for NODE_ENV in the env config to use a fail-safe
value (change .default("development") to .default("production") or remove the
default to make it required) so the app runs in production-safe mode when
NODE_ENV is omitted; locate the NODE_ENV definition using the z.enum([...]) call
in the env.ts file and replace the default accordingly.
- Around line 9-18: The env schema currently allows empty AWS_* and bucket
strings and accepts any numeric-looking PORT; update the schema in env.ts so
AWS_REGION, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_S3_BUCKET_NAME use
z.string().min(1) (to reject empty strings) and tighten PORT to a validated
integer within 0–65535 (e.g., replace the regex/transform pipeline with a
z.number().int().min(0).max(65535) or parse-and-refine approach) ensuring the
identifiers PORT, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION, and
AWS_S3_BUCKET_NAME are the ones updated.
In
`@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/controllers/upload.controller.ts`:
- Around line 37-41: The controller currently only checks presence of
req.body.key; update the validation to reject non-string or empty-string values:
verify typeof key === 'string' && key.trim().length > 0 before proceeding (in
the same controller handling the delete flow, where const { key } = req.body is
declared), and if the check fails call next(ApiError.badRequest("File key must
be a non-empty string")) so objects/numbers are rejected explicitly.
- Around line 27-29: The multi-upload uses Promise.all in upload.controller.ts
which leaves already-uploaded files in S3 if one upload fails; change the logic
to use Promise.allSettled over files.map(file => uploadToS3(file,
"uploads/files")), collect successful S3UploadResult entries, and if any upload
failed call a cleanup routine (e.g., deleteFromS3 or a new helper) to remove the
successfully uploaded objects, then throw/return the original error; ensure
uploadToS3, S3UploadResult, files, and the cleanup helper are referenced so you
remove the correct keys/paths for compensation.
In
`@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/services/s3.service.ts`:
- Around line 20-21: The current key/URL generation can produce unsafe
characters from file.originalname; sanitize the extension and URL-encode key
segments: when computing ext (the variable assigned from
file.originalname.split(".").pop()), normalize by lowercasing, strip anything
but [a-z0-9] (fallback to "bin" if empty), and then build the key using encoded
segments (e.g., encodeURIComponent(folder) and
encodeURIComponent(`${randomUUID()}.${ext}`)) instead of raw interpolation;
likewise, when returning the public URL (the code that combines the S3 bucket
base with the key), ensure the key portion is encoded (use encodeURI or
encodeURIComponent on the key segment) so the returned URL is safe.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/app.upload-file.test.ts`:
- Around line 1-20: The file is misnamed with a `.test.ts` suffix while
containing actual app bootstrap code (express Application instance `app`,
middleware `errorHandler`, route mount `Routes`, and `app.listen` using
`env.PORT`); rename the file to a non-test name such as `app.ts`, `server.ts`,
or `index.ts` and update any imports or exports that reference this module so
test runners won't treat it as a test file and the application bootstrap remains
correctly discoverable.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.service.ts`:
- Around line 20-21: The key construction in upload.service.ts uses ext =
file.originalname.split(".").pop() which yields wrong values for filenames
without dots or for dotfiles; update the logic in the upload method (the ext,
key, folder, randomUUID() usage) to derive the extension defensively (e.g., use
path.extname or a regex), treat empty or missing extensions as “no extension”
(omit the trailing dot) or supply a safe default, and ensure keys for dotfiles
(e.g., ".gitignore") do not end with a dangling dot—construct key so it becomes
`${folder}/${randomUUID()}` when there's no valid ext, otherwise
`${folder}/${randomUUID()}.${ext}`.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/configs/env.ts`:
- Around line 15-19: The environment schema currently allows empty R2 credential
strings; update the zod schema in configs/env.ts for R2_ACCOUNT_ID,
R2_ACCESS_KEY_ID, R2_SECRET_ACCESS_KEY, and R2_BUCKET_NAME to reject empty
strings (e.g., replace z.string() with z.string().nonempty() or
z.string().min(1)) so boot-time validation fails for blank credentials, leaving
R2_PUBLIC_URL as z.string().url().
- Line 9: PORT validation currently only checks digits via z.string().regex and
can allow out-of-range values; update the PORT schema (the PORT entry in
configs/env.ts) to enforce numeric bounds by parsing/coercing to a number and
validating range (1–65535) and integer-ness — e.g. use z.preprocess to convert
the string to a number and then z.number().int().min(1).max(65535) or use
z.coerce.number().int().min(1).max(65535) so invalid ports like 70000 are
rejected.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/controllers/upload.controller.ts`:
- Around line 27-29: Promise.all is used for batch uploads so a single failure
leaves already-uploaded objects orphaned; change the logic in the upload handler
that currently calls Promise.all(files.map(file => uploadToR2(file,
"uploads/files"))) to use Promise.allSettled to collect per-file results
(success/failure), detect if any upload failed, and if so perform a rollback by
deleting all successfully uploaded objects (use the same upload key/path
returned by uploadToR2 or call the R2 delete helper such as deleteFromR2 or
bucket.delete if available) before returning the error response; ensure the
final returned result is either the array of upload results on full success or a
failure response after cleanup.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/error-handler.ts`:
- Around line 19-23: The error handler only treats ApiError and falls back to
500 for everything else; add explicit handling for Multer errors by checking for
MulterError (from the 'multer' package) before the ApiError branch (or
immediately after) and mapping err.code === 'LIMIT_FILE_SIZE' to statusCode 413,
other MulterError codes to 400, and set message = err.message and errors to
include any err.field or err.code info; ensure the handler references the
MulterError type/check (e.g., using instanceof or code property on err) so
file-upload validation failures return appropriate 4xx responses instead of 500.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.ts`:
- Around line 15-27: The current middleware uses multer.memoryStorage() (see
storage and upload constants and upload.array usage) which buffers full files in
RAM; replace this by removing memoryStorage and instead implement a streaming
upload path that pipes each file stream directly to Cloudflare R2 via the AWS
S3/compatible SDK (or a custom multer storage engine) so files are sent to R2 as
they are received; update the upload setup to use that streaming storage engine
or a middleware that iterates files from the multipart stream and calls
S3.putObject/upload for each file (keep fileFilter and limits logic but ensure
no in-memory buffering or diskStorage is used).
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/r2.service.ts`:
- Around line 48-56: The DeleteObjectsCommand response is ignored so partial
failures (returned in result.Errors when Quiet: true) are missed; change the
await r2Client.send(new DeleteObjectsCommand({...})) call to capture the result
(e.g., const result = await r2Client.send(...)) and inspect result.Errors (or
result.Deleted vs Errors) to detect per-key failures for the bucket
env.R2_BUCKET_NAME and the keys produced by keys.map(Key => ({ Key })); if any
Errors are present, surface them (throw an error or log and return failure)
identifying which Key(s) failed so callers can handle partial deletions.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/api-response.ts`:
- Line 8: The ApiResponse<T> interface declares data?: T | null (and errors?:
... | null) but the response builder only checks !== undefined when spreading,
so explicit nulls get serialized; change the spreading conditions in the
response builder (the code that currently checks data !== undefined and errors
!== undefined) to use != null so both null and undefined are excluded, or
alternatively remove | null from the ApiResponse<T> fields if you want null to
be serialized; adjust the checks for the data and errors fields in the response
construction logic accordingly (referencing ApiResponse<T>, the data and errors
fields, and the response builder/spread logic).
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/logger.ts`:
- Line 12: The translateTime format in the pino-pretty configuration is using
the wrong month/minute tokens; update the translateTime string used in the
logger config (the property named translateTime in the pino/pino-pretty setup
inside logger.ts) from "yyyy-mm-dd HH:MM:ss" to the correct "yyyy-MM-dd
HH:mm:ss" so months use capital MM and minutes use lowercase mm; locate the
logger config/pinoPretty options in logger.ts and replace the format string
accordingly.
---
Nitpick comments:
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.routes.ts`:
- Line 13: The DELETE route currently uses a body payload (router.delete("/",
deleteFile)); change the route to accept the file identifier as a path param
(e.g., router.delete("/:key", deleteFile)) and update the deleteFile controller
to read the identifier from req.params.key instead of req.body.key; also update
any callers/tests/clients to call DELETE /:key (or use query param) and ensure
validation/error handling still applies for missing/invalid key in the
deleteFile handler.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/upload-file.ts`:
- Around line 17-21: Move MIME-type rejection into the multer fileFilter by
passing an ApiError to the multer callback instead of silently returning false;
in the fileFilter function (named fileFilter) replace the current cb(null,
false) branch with cb(ApiError.badRequest("...invalid file type...")) so multer
forwards the error to your error-handling middleware, and then remove or
simplify the controller's explicit if (!req.file) check since validation is
handled earlier.
In
`@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-response.ts`:
- Line 47: Replace the magic number default for the statusCode property with the
named constant used elsewhere: change the default in the ApiResponse (the
declaration where statusCode: StatusCode = 200) to use STATUS_CODES.OK so it
matches the ok/created helpers and improves consistency.
In
`@packages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/async-handler.ts`:
- Around line 9-15: The AsyncHandler function lacks an explicit return type;
update the AsyncHandler signature to declare its return type explicitly (e.g., a
function type that takes Request, Response, NextFunction and returns void or an
Express RequestHandler) so the exported symbol has clear typing; locate the
AsyncHandler function and annotate its return type using the existing
AsyncRouteHandler, Request, Response, and NextFunction types to ensure callers
and TypeScript inferencing see the correct API.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.controller.ts`:
- Line 21: The code unsafely asserts req.files as Express.Multer.File[]; replace
this brittle assertion with a runtime check: verify Array.isArray(req.files)
(and optionally that items have expected properties like
originalname/mimetype/buffer) before treating them as files, and return a
400/error if the check fails; update the handler that uses upload.array("files",
10) to narrow the type only after the runtime validation (e.g., assign to files
after the guard) so changes to middleware won’t cause a runtime crash.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.service.ts`:
- Around line 47-57: The function deleteManyFromR2 is defined but unused; either
remove it or wire it up as a bulk-delete API: if you want to keep it, add a
controller method (e.g., UploadController.deleteMany or a new handler in
upload.controller.ts) that validates req.body.keys is a non-empty string[] and
calls deleteManyFromR2(keys), then register that handler on the upload router
(e.g., POST /upload/delete-many) and return an appropriate status (204 on
success or 400/500 on error). If you prefer removal, delete deleteManyFromR2
from upload.service.ts and any unused imports to avoid dead code. Ensure you
reference deleteManyFromR2 when implementing or removing to keep changes
targeted.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/upload-file.ts`:
- Around line 17-21: The fileFilter currently calls cb(null, false) for
unsupported MIME types which silently drops the file; replace that with calling
the multer callback with an ApiError bad request so the client receives a 400.
Update the fileFilter (multer.Options["fileFilter"]) to call cb(new
ApiError.badRequest(`Unsupported file type: ${file.mimetype}`)) for types not in
ALLOWED_FILE_TYPES, and ensure ApiError is imported/available in this module so
the error propagates as a 400 instead of a silent “no file.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf91e0a3-5407-4d89-9435-7b82a6b875ef
📒 Files selected for processing (56)
.husky/pre-commitpackages/registry/component/file-upload.jsonpackages/templates/node/express/component/file-upload/aws-s3/feature/src/app.upload-file.test.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.controller.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.routes.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.service.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/routes/index.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/env.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/s3.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/constants/status-codes.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/errors/api-error.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/error-handler.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/upload-file.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/utils/api-response.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/utils/async-handler.tspackages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/utils/logger.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/app.upload-file.test.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/configs/env.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/configs/s3.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/constants/status-codes.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/controllers/upload.controller.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/middlewares/error-handler.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/middlewares/upload-file.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/routes/upload.routes.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/services/s3.service.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-error.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/api-response.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/async-handler.tspackages/templates/node/express/component/file-upload/aws-s3/mvc/src/utils/logger.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/app.upload-file.test.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.controller.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.routes.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/modules/upload/upload.service.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/routes/index.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/configs/env.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/configs/r2.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/constants/status-codes.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/errors/api-error.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/error-handler.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/middlewares/upload-file.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/utils/api-response.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/utils/async-handler.tspackages/templates/node/express/component/file-upload/cloudflare-r2/feature/src/shared/utils/logger.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/app.upload-file.test.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/configs/env.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/configs/r2.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/constants/status-codes.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/controllers/upload.controller.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/error-handler.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/routes/upload.routes.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/r2.service.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/api-error.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/api-response.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/async-handler.tspackages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/logger.ts
| const app: Application = express(); | ||
|
|
||
| const PORT = env.PORT; | ||
|
|
||
| app.use(express.urlencoded({ extended: true })); | ||
| app.use(express.json()); | ||
|
|
||
| app.use("/api", Routes); | ||
|
|
||
| app.use(errorHandler); | ||
|
|
||
| app.listen(PORT, () => { | ||
| logger.info(`Server is running on http://localhost:${PORT}`); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*app.upload-file*" -o -name "*test.ts" | grep -i upload | head -20Repository: AkkalDhami/servercn
Length of output: 865
🏁 Script executed:
git ls-files | grep -E "(upload|aws)" | head -20Repository: AkkalDhami/servercn
Length of output: 2140
🏁 Script executed:
git ls-files | grep "packages/templates" | head -20Repository: AkkalDhami/servercn
Length of output: 2232
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/feature/src/app.upload-file.test.tsRepository: AkkalDhami/servercn
Length of output: 723
🏁 Script executed:
find packages/templates/node/express/component/file-upload/aws-s3/feature/src -type f -name "*.ts" | head -20Repository: AkkalDhami/servercn
Length of output: 1481
🏁 Script executed:
ls -la packages/templates/node/express/component/file-upload/aws-s3/feature/src/Repository: AkkalDhami/servercn
Length of output: 410
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/feature/src/routes/index.tsRepository: AkkalDhami/servercn
Length of output: 305
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/env.tsRepository: AkkalDhami/servercn
Length of output: 1121
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/mvc/src/app.upload-file.test.tsRepository: AkkalDhami/servercn
Length of output: 730
Decouple app creation from server startup.
Line 18 executes app.listen() at module import time. Since this is a template file, any developer importing or copying this code gets unwanted server startup side effects. This pattern breaks composability and makes it unsafe to import the app in other modules. Export the app and move listen() to a dedicated entrypoint.
Suggested refactor
const app: Application = express();
@@
-app.listen(PORT, () => {
- logger.info(`Server is running on http://localhost:${PORT}`);
-});
+export default app;// server.ts (new bootstrap entrypoint)
import app from "./app.upload-file.test";
import env from "./shared/configs/env";
import { logger } from "./shared/utils/logger";
app.listen(env.PORT, () => {
logger.info(`Server is running on http://localhost:${env.PORT}`);
});📝 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.
| const app: Application = express(); | |
| const PORT = env.PORT; | |
| app.use(express.urlencoded({ extended: true })); | |
| app.use(express.json()); | |
| app.use("/api", Routes); | |
| app.use(errorHandler); | |
| app.listen(PORT, () => { | |
| logger.info(`Server is running on http://localhost:${PORT}`); | |
| }); | |
| const app: Application = express(); | |
| const PORT = env.PORT; | |
| app.use(express.urlencoded({ extended: true })); | |
| app.use(express.json()); | |
| app.use("/api", Routes); | |
| app.use(errorHandler); | |
| export default app; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/app.upload-file.test.ts`
around lines 7 - 20, The module currently calls app.listen(PORT) at import time
(using app, Routes, errorHandler, env.PORT, logger), causing side effects;
refactor by removing the app.listen(...) call from this file, export the
configured Express application (export default app), and move the server startup
into a new bootstrap entrypoint (e.g., server.ts) that imports the exported app,
reads env.PORT, and calls app.listen with the logger call; ensure Routes and
errorHandler remain wired to the exported app.
| const { key } = req.body; | ||
|
|
||
| if (!key) { | ||
| return next(ApiError.badRequest("File key is required")); | ||
| } | ||
|
|
||
| await deleteFromS3(key); |
There was a problem hiding this comment.
Validate and scope deletion keys before calling S3 delete.
Line 39 only checks presence, so non-string values or keys outside your upload namespace can pass through and be deleted at Line 43. Validate type and enforce a safe prefix (for example uploads/files/).
Suggested hardening
const { key } = req.body;
- if (!key) {
- return next(ApiError.badRequest("File key is required"));
+ if (typeof key !== "string" || !key.startsWith("uploads/files/")) {
+ return next(ApiError.badRequest("Valid file key is required"));
}
await deleteFromS3(key);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.controller.ts`
around lines 37 - 43, The controller currently only checks presence of
req.body.key before calling deleteFromS3; update the logic to validate that key
is a string and that it begins with a safe upload prefix (e.g. "uploads/files/")
before calling deleteFromS3, returning next(ApiError.badRequest(...)) for
non-string or disallowed-prefix values; use a named constant (e.g.
UPLOAD_PREFIX) or config value to check key.startsWith(UPLOAD_PREFIX) so keys
outside the upload namespace are rejected.
| export const deleteManyFromS3 = async (keys: string[]): Promise<void> => { | ||
| await s3Client.send( | ||
| new DeleteObjectsCommand({ | ||
| Bucket: env.AWS_S3_BUCKET_NAME, | ||
| Delete: { | ||
| Objects: keys.map(Key => ({ Key })), | ||
| Quiet: true | ||
| } | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "upload.service.ts" | head -20Repository: AkkalDhami/servercn
Length of output: 966
🏁 Script executed:
cat -n "packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.service.ts" 2>/dev/null | head -70Repository: AkkalDhami/servercn
Length of output: 1802
🌐 Web query:
AWS S3 DeleteObjectsCommand response Errors array partial failure
💡 Result:
The AWS S3 DeleteObjects API operation is designed to handle multiple object deletions in a single request, and it inherently supports partial failures [1][2]. Because the operation processes each object individually, some objects may be deleted successfully while others encounter errors [1][2]. When a partial failure occurs, the S3 service returns an HTTP 200 OK response [1][2]. This status code indicates that the overall request was processed, but the response body contains detailed information about the outcome of each individual object deletion [1][2]. Key aspects of handling these responses include: 1. Response Structure: The response body includes two primary containers: - Deleted: A list of objects that were successfully deleted [1][3]. - Errors: A list of objects that failed to delete, including the specific error code (e.g., AccessDenied, NoSuchKey) and message for each [4][1][3]. 2. Response Modes: - Verbose (Default): The response includes both the Deleted and Errors lists [4][1]. - Quiet: The response only includes the Errors list [4][1]. If all deletions are successful, the response body is empty [4][1]. 3. Implementation Note: When using AWS SDKs, you must inspect the response object returned by the DeleteObjectsCommand [3]. If your application logic assumes that a 200 OK response means every object was deleted, it may fail to detect partial failures [3]. You should explicitly check the Errors collection in the response to identify and handle any failed deletions [3]. Note that if the entire request fails (e.g., due to authentication issues, invalid bucket names, or MFA errors), the operation will return a standard HTTP error status code (such as 403 or 404) rather than a 200 OK with an Errors array [1][2][5].
Citations:
- 1: https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html
- 2: https://docs.aws.amazon.com/goto/SdkForJavaScriptV3/s3-2006-03-01/DeleteObjects
- 3: https://docs.aws.amazon.com/sdkfornet/v3/apidocs/items/S3/TDeleteObjectsResponse.html
- 4: https://docs.aws.amazon.com/cli/latest/reference/s3api/delete-objects.html
- 5: https://docs.aws.amazon.com/AmazonS3/latest/userguide/DeletingObjects.html
Handle partial failures from DeleteObjectsCommand (and empty input).
DeleteObjectsCommand returns HTTP 200 even when individual object deletions fail; failures are reported in the Errors array of the response. Current code ignores the response entirely, silently missing partial failures. Also short-circuit when keys is empty to avoid unnecessary API calls.
Suggested fix
export const deleteManyFromS3 = async (keys: string[]): Promise<void> => {
- await s3Client.send(
+ if (keys.length === 0) return;
+
+ const result = await s3Client.send(
new DeleteObjectsCommand({
Bucket: env.AWS_S3_BUCKET_NAME,
Delete: {
Objects: keys.map(Key => ({ Key })),
Quiet: true
}
})
);
+
+ if (result.Errors?.length) {
+ throw new Error(
+ `Failed to delete some objects: ${result.Errors.map(e => e.Key).join(", ")}`
+ );
+ }
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/modules/upload/upload.service.ts`
around lines 47 - 56, The deleteManyFromS3 helper currently always calls
DeleteObjectsCommand and ignores its response, which can hide per-object
failures and also unnecessarily calls S3 when keys is empty; update
deleteManyFromS3 to short-circuit and return immediately when keys.length === 0,
then await the DeleteObjectsCommand response, inspect the returned Errors array
(response.Errors) and handle any entries (e.g., throw a descriptive error or log
and rethrow) so partial failures are surfaced; reference s3Client,
DeleteObjectsCommand and deleteManyFromS3 when implementing this change.
| AWS_REGION: z.string(), | ||
| AWS_ACCESS_KEY_ID: z.string(), | ||
| AWS_SECRET_ACCESS_KEY: z.string(), | ||
| AWS_S3_BUCKET_NAME: z.string() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "env.ts" | grep -E "aws-s3.*shared.*configs"Repository: AkkalDhami/servercn
Length of output: 160
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/env.tsRepository: AkkalDhami/servercn
Length of output: 1121
🏁 Script executed:
rg "AWS_REGION|AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY|AWS_S3_BUCKET_NAME" packages/templates/node/express/component/file-upload/aws-s3/feature/src --type ts -A 2 -B 1Repository: AkkalDhami/servercn
Length of output: 3870
🏁 Script executed:
rg "AWS_REGION|AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY|AWS_S3_BUCKET_NAME" packages/templates/node/express/component/file-upload/aws-s3/feature/src --type ts | grep -E "(\.length|\.trim|min\(|required)" | head -20Repository: AkkalDhami/servercn
Length of output: 45
Reject empty AWS config values during startup validation.
Lines 15-18 currently accept "" values, which pass boot-time validation and fail later at runtime during S3 operations. Enforce non-empty strings with .min(1).
Suggested schema tightening
- AWS_REGION: z.string(),
- AWS_ACCESS_KEY_ID: z.string(),
- AWS_SECRET_ACCESS_KEY: z.string(),
- AWS_S3_BUCKET_NAME: z.string()
+ AWS_REGION: z.string().trim().min(1, "AWS_REGION is required"),
+ AWS_ACCESS_KEY_ID: z.string().trim().min(1, "AWS_ACCESS_KEY_ID is required"),
+ AWS_SECRET_ACCESS_KEY: z.string().trim().min(1, "AWS_SECRET_ACCESS_KEY is required"),
+ AWS_S3_BUCKET_NAME: z.string().trim().min(1, "AWS_S3_BUCKET_NAME is required")📝 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.
| AWS_REGION: z.string(), | |
| AWS_ACCESS_KEY_ID: z.string(), | |
| AWS_SECRET_ACCESS_KEY: z.string(), | |
| AWS_S3_BUCKET_NAME: z.string() | |
| AWS_REGION: z.string().trim().min(1, "AWS_REGION is required"), | |
| AWS_ACCESS_KEY_ID: z.string().trim().min(1, "AWS_ACCESS_KEY_ID is required"), | |
| AWS_SECRET_ACCESS_KEY: z.string().trim().min(1, "AWS_SECRET_ACCESS_KEY is required"), | |
| AWS_S3_BUCKET_NAME: z.string().trim().min(1, "AWS_S3_BUCKET_NAME is required") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/configs/env.ts`
around lines 15 - 18, The env schema currently allows empty AWS values because
AWS_REGION, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_S3_BUCKET_NAME are
defined as z.string() which accepts "" — update those schema entries to require
non-empty strings by using z.string().min(1) for each of AWS_REGION,
AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and AWS_S3_BUCKET_NAME in the env.ts
config so boot-time validation rejects empty values.
| let statusCode = 500; | ||
| let message = "Internal server error"; | ||
| let errors: unknown; | ||
|
|
||
| if (err instanceof ApiError) { | ||
| statusCode = err.statusCode; | ||
| message = err.message; | ||
| errors = err.errors; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages -name "error-handler.ts" -type f | head -20Repository: AkkalDhami/servercn
Length of output: 2293
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/error-handler.tsRepository: AkkalDhami/servercn
Length of output: 1237
🏁 Script executed:
cat packages/templates/node/express/component/file-upload/aws-s3/feature/package.jsonRepository: AkkalDhami/servercn
Length of output: 177
🏁 Script executed:
find packages/templates/node/express/component/file-upload/aws-s3 -name "package.json" -type fRepository: AkkalDhami/servercn
Length of output: 45
🏁 Script executed:
cat package.json | head -50Repository: AkkalDhami/servercn
Length of output: 1541
🏁 Script executed:
rg "multer" package.json | head -20Repository: AkkalDhami/servercn
Length of output: 118
🏁 Script executed:
rg "multer" packages/templates/node/express/component/file-upload/aws-s3/feature --type tsRepository: AkkalDhami/servercn
Length of output: 655
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/upload-file.tsRepository: AkkalDhami/servercn
Length of output: 850
🌐 Web query:
multer.MulterError error codes LIMIT_FILE_SIZE HTTP status
💡 Result:
When using Multer, a LIMIT_FILE_SIZE error does not automatically set a specific HTTP status code [1]. By default, if the error is passed to the Express error-handling middleware, it will typically result in a 500 Internal Server Error unless you explicitly handle it [2][1]. To return a more appropriate HTTP status code, such as 413 Request Entity Too Large, you must catch the error within your route handler or a custom error-handling middleware and set the status manually [3][4][5]. Recommended Implementation: You can handle the error by wrapping the Multer middleware in a callback function within your route [3][2]. This allows you to inspect the error object and respond with the desired status code [1][5]. app.post('/upload', (req, res) => { upload.single('file')(req, res, (err) => { if (err instanceof multer.MulterError && err.code === 'LIMIT_FILE_SIZE') { return res.status(413).json({ error: 'File too large' }); } else if (err) { return res.status(500).json({ error: err.message }); } res.send('Upload successful'); }); }); Alternatively, you can use a global error-handling middleware, though you must ensure the Multer middleware is executed before it [4][5]. Note that if you see a 413 error before your application code is reached, it may be coming from a reverse proxy (like Nginx or OpenResty) rather than Multer itself [6].
Citations:
- 1: fix: correct HTTP status codes for upload limit errors lopatnov/express-reverse-proxy#32
- 2: https://multerguide.vercel.app/blogs/multer-error-handling/
- 3: https://stackoverflow.com/questions/78737142/multererror-file-too-large-how-to-handle-this-error
- 4: https://stackoverflow.com/questions/65156525/handle-multer-filesize-error-when-uploading-large-files
- 5: https://www.codestudy.net/blog/how-to-limit-upload-file-size-in-express-js/
- 6: Error: 413 Request Entity Too Large expressjs/multer#1166
Map multer-originated upload errors to 4xx instead of 500.
Non-ApiError failures currently default to 500, which misclassifies client upload faults (such as file size violations) as server errors. Since this component uses multer with file size limits (5MB), multer.MulterError instances thrown during validation need explicit handling to return appropriate HTTP status codes (413 for oversized files, 400 for other validation failures).
💡 Suggested patch
+import multer from "multer";
import { Request, Response, NextFunction } from "express";
import env from "../configs/env";
import { ApiError } from "../errors/api-error";
import { logger } from "../utils/logger";
@@
if (err instanceof ApiError) {
statusCode = err.statusCode;
message = err.message;
errors = err.errors;
+ } else if (err instanceof multer.MulterError) {
+ statusCode =
+ err.code === "LIMIT_FILE_SIZE" ? 413 : 400;
+ message =
+ err.code === "LIMIT_FILE_SIZE"
+ ? "File is too large"
+ : err.message;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/aws-s3/feature/src/shared/middlewares/error-handler.ts`
around lines 15 - 23, The middleware currently treats non-ApiError errors as
500; update the error-handler (the function that inspects err in
error-handler.ts) to special-case multer.MulterError: if err is an instance of
multer.MulterError, set statusCode to 413 and a clear message when err.code ===
'LIMIT_FILE_SIZE' (file too large), otherwise set statusCode to 400 for other
multer validation failures and include err.message/details in the errors
payload; preserve existing ApiError handling (err instanceof ApiError) and keep
default 500 for truly unexpected errors.
| if (err instanceof ApiError) { | ||
| statusCode = err.statusCode; | ||
| message = err.message; | ||
| errors = err.errors; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*/packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/error-handler.ts" -type fRepository: AkkalDhami/servercn
Length of output: 170
🏁 Script executed:
cat -n ./packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/error-handler.tsRepository: AkkalDhami/servercn
Length of output: 1236
🏁 Script executed:
find ./packages/templates/node/express/component/file-upload/cloudflare-r2/mvc -type f \( -name "*.ts" -o -name "*.js" \) | head -20Repository: AkkalDhami/servercn
Length of output: 1372
🏁 Script executed:
cat -n ./packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.tsRepository: AkkalDhami/servercn
Length of output: 850
🏁 Script executed:
cat -n ./packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/routes/upload.routes.tsRepository: AkkalDhami/servercn
Length of output: 562
🏁 Script executed:
cat -n ./packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/app.upload-file.test.ts | head -100Repository: AkkalDhami/servercn
Length of output: 730
Add handling for Multer errors in the error handler.
The error handler currently defaults all non-ApiError exceptions to 500 status codes. When Multer throws errors (e.g., file size exceeds 5MB limit, unsupported file type), they should return appropriate 4xx client error codes (413 for file size, 400 for validation failures) instead of 500.
Suggested fix
import { Request, Response, NextFunction } from "express";
+import multer from "multer";
import env from "../configs/env";
import { ApiError } from "../utils/api-error";
import { logger } from "../utils/logger";
@@
if (err instanceof ApiError) {
statusCode = err.statusCode;
message = err.message;
errors = err.errors;
+ } else if (err instanceof multer.MulterError) {
+ statusCode = err.code === "LIMIT_FILE_SIZE" ? 413 : 400;
+ message = err.message;
}📝 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.
| if (err instanceof ApiError) { | |
| statusCode = err.statusCode; | |
| message = err.message; | |
| errors = err.errors; | |
| } | |
| if (err instanceof ApiError) { | |
| statusCode = err.statusCode; | |
| message = err.message; | |
| errors = err.errors; | |
| } else if (err instanceof multer.MulterError) { | |
| statusCode = err.code === "LIMIT_FILE_SIZE" ? 413 : 400; | |
| message = err.message; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/error-handler.ts`
around lines 19 - 23, The error handler only treats ApiError and falls back to
500 for everything else; add explicit handling for Multer errors by checking for
MulterError (from the 'multer' package) before the ApiError branch (or
immediately after) and mapping err.code === 'LIMIT_FILE_SIZE' to statusCode 413,
other MulterError codes to 400, and set message = err.message and errors to
include any err.field or err.code info; ensure the handler references the
MulterError type/check (e.g., using instanceof or code property on err) so
file-upload validation failures return appropriate 4xx responses instead of 500.
| const storage = multer.memoryStorage(); | ||
|
|
||
| const fileFilter: multer.Options["fileFilter"] = (_req, file, cb) => { | ||
| if (!ALLOWED_FILE_TYPES.includes(file.mimetype)) { | ||
| return cb(null, false); | ||
| } | ||
| cb(null, true); | ||
| }; | ||
|
|
||
| const upload = multer({ | ||
| storage, | ||
| limits: { fileSize: MAX_FILE_SIZE }, | ||
| fileFilter |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine its content
find . -path "*/packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.ts" -type fRepository: AkkalDhami/servercn
Length of output: 168
🏁 Script executed:
# List the file structure to understand the context
find . -path "*/packages/templates/node/express/component/file-upload/cloudflare-r2*" -type f | head -20Repository: AkkalDhami/servercn
Length of output: 2216
🏁 Script executed:
# Search for the upload-file middleware configuration
rg -A 30 "const storage = multer.memoryStorage" --type tsRepository: AkkalDhami/servercn
Length of output: 50375
🏁 Script executed:
# Search for upload.array or any upload configuration with limits
rg "upload\.array|fileSize.*MAX_FILE_SIZE|MAX_FILE_SIZE\s*=" --type tsRepository: AkkalDhami/servercn
Length of output: 8833
🏁 Script executed:
# Check for the context - look for README or package.json in the template
find . -path "*/packages/templates/node/express/component/file-upload/cloudflare-r2*" -name "README*" -o -name "package.json" | xargs cat 2>/dev/null | head -50Repository: AkkalDhami/servercn
Length of output: 1556
🏁 Script executed:
cat -n packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.tsRepository: AkkalDhami/servercn
Length of output: 850
🏁 Script executed:
# Check if there's any direct streaming to R2 elsewhere in the codebase
rg -l "busboy|stream.*R2|createReadStream" --type ts packages/templates/node/express/component/file-upload/cloudflare-r2/Repository: AkkalDhami/servercn
Length of output: 45
🏁 Script executed:
# Check how files are actually uploaded to R2 in the service/controller
cat -n packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/controllers/upload.controller.tsRepository: AkkalDhami/servercn
Length of output: 1844
🏁 Script executed:
# Check the R2 service to see how files are uploaded
cat -n packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/r2.service.ts | head -100Repository: AkkalDhami/servercn
Length of output: 1722
🏁 Script executed:
# Check if the service supports streaming
rg -A 20 "uploadToR2\|R2UploadResult" packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/ --type ts | head -60Repository: AkkalDhami/servercn
Length of output: 45
Avoid in-memory buffering for multi-file uploads in production Cloudflare R2 template.
On Line 15, memoryStorage() buffers complete files in RAM. With upload.array(..., 10) and 5MB max per file, a single request can buffer ~50MB; concurrent requests risk exhausting memory. However, for Cloudflare R2, diskStorage is suboptimal (adds I/O overhead by writing then re-reading from disk). Instead, use a streaming approach that pipes request data directly to R2 via the AWS SDK, bypassing buffering entirely.
Suggested direction
-const storage = multer.memoryStorage();
+// Use streaming directly to R2 to avoid buffering
+// Consider using busboy or native stream handling with
+// AWS SDK's PutObjectCommand accepting readable streams
+// instead of complete buffers
+const storage = multer.memoryStorage(); // Temporary until streaming refactorAlternatively, implement a custom storage engine or middleware that streams directly to R2 without intermediate buffering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/middlewares/upload-file.ts`
around lines 15 - 27, The current middleware uses multer.memoryStorage() (see
storage and upload constants and upload.array usage) which buffers full files in
RAM; replace this by removing memoryStorage and instead implement a streaming
upload path that pipes each file stream directly to Cloudflare R2 via the AWS
S3/compatible SDK (or a custom multer storage engine) so files are sent to R2 as
they are received; update the upload setup to use that streaming storage engine
or a middleware that iterates files from the multipart stream and calls
S3.putObject/upload for each file (keep fileFilter and limits logic but ensure
no in-memory buffering or diskStorage is used).
| await r2Client.send( | ||
| new DeleteObjectsCommand({ | ||
| Bucket: env.R2_BUCKET_NAME, | ||
| Delete: { | ||
| Objects: keys.map(Key => ({ Key })), | ||
| Quiet: true | ||
| } | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the target file
fd -type f "r2.service.ts" packages/templates/Repository: AkkalDhami/servercn
Length of output: 234
🏁 Script executed:
# If found, examine its contents
cat -n packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/r2.service.tsRepository: AkkalDhami/servercn
Length of output: 1722
🌐 Web query:
DeleteObjectsCommand AWS S3 response errors partial failure handling
💡 Result:
When using the S3 DeleteObjectsCommand, it is important to understand that a single request can result in a partial failure, where some objects are successfully deleted while others encounter errors [1][2]. Because the operation processes multiple keys, the HTTP response status code (typically 200 OK) indicates that the request itself was processed, but it does not guarantee that every individual object deletion succeeded [2][3]. To handle partial failures effectively, you must inspect the response body [1][2]. Response Modes The DeleteObjects operation supports two response modes [1][2]: 1. Verbose (Default): The response includes a list of both successfully deleted objects and any objects that failed to delete [1][2]. 2. Quiet: The response only includes information about objects that encountered an error [1][2]. Handling Partial Failures To ensure your application correctly handles partial failures, follow these steps: 1. Inspect the Response: After sending the command, parse the response object [2][3]. Look for an Errors element (or DeleteErrors list in some SDKs) [2][3]. 2. Iterate Through Errors: If the Errors element is present and contains items, iterate through them to identify which specific keys failed and why (e.g., AccessDenied, NoSuchKey) [2][3]. 3. Implement Retry Logic: For transient errors (such as 500 Internal Server Error or 503 Slow Down), implement a retry mechanism with exponential backoff [4]. For permanent errors (like AccessDenied), you should log the failure and handle it according to your application's business logic [2][3]. Note that if the entire request fails (e.g., due to an invalid MFA token or a non-existent bucket), the SDK will typically throw an exception rather than returning a partial success response [1][3]. Always wrap your DeleteObjectsCommand calls in try-catch blocks to handle these request-level failures [5][6][3].
Citations:
- 1: https://docs.aws.amazon.com/goto/SdkForJavaScriptV3/s3-2006-03-01/DeleteObjects
- 2: https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html
- 3: https://docs.aws.amazon.com/sdkfornet/v3/apidocs/items/S3/TDeleteObjectsResponse.html
- 4: s3 DeleteObjectsCommand fails with 500 aws/aws-sdk-js-v3#2290
- 5: https://docs.aws.amazon.com/code-library/latest/ug/s3_example_s3_DeleteObjects_section.html
- 6: https://docs.aws.amazon.com/code-library/latest/ug/s3_example_s3_Scenario_DeleteAllObjects_section.html
🏁 Script executed:
# Check AWS SDK v3 for DeleteObjectsCommand response types
cd packages/templates/node/express/component/file-upload/cloudflare-r2/mvc && cat package.json | grep -A5 -B5 "@aws-sdk"Repository: AkkalDhami/servercn
Length of output: 108
🌐 Web query:
AWS SDK v3 DeleteObjectsCommandOutput Errors type typescript
💡 Result:
In the AWS SDK for JavaScript v3, the DeleteObjectsCommandOutput interface includes an Errors field that captures failures for individual objects within a batch delete request [1]. Because the DeleteObjects operation processes multiple objects, it is possible for the overall HTTP request to succeed (returning a 200 OK status) while individual object deletions fail [2]. The Errors field is defined as an array of objects, where each object contains the following properties [1]: - Key: The key of the object that failed to delete [1]. - VersionId: The version ID of the object, if applicable [1]. - Code: A string representing the error code (e.g., AccessDenied, NoSuchKey) [1]. - Message: A descriptive string explaining the error [1]. In TypeScript, this is represented as: interface DeleteObjectsCommandOutput { Deleted?: { Key?: string; VersionId?: string; DeleteMarker?: boolean; DeleteMarkerVersionId?: string; }[]; Errors?: { Key?: string; VersionId?: string; Code?: string; Message?: string; }[]; RequestCharged?: string; $metadata: any; } It is important to note that the SDK does not automatically throw an exception if individual objects fail to delete within a batch request [2]. You must explicitly check the Errors array in the response to determine if any specific deletions failed [1][2]. If the Errors array is present and contains items, those specific operations were unsuccessful despite the overall command's success [1][2].
Citations:
- 1: https://docs.aws.amazon.com/goto/SdkForJavaScriptV3/s3-2006-03-01/DeleteObjects
- 2: https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html
Check DeleteObjects response for per-key failures.
The response from DeleteObjectsCommand is ignored, which silently masks partial failures. Even when the request succeeds (HTTP 200), individual object deletions can fail and be reported in the Errors array of the response. With Quiet: true, only failed deletions are returned. The code must inspect result.Errors to detect which keys failed to delete.
Suggested fix:
Capture and validate response
export const deleteManyFromR2 = async (keys: string[]): Promise<void> => {
- await r2Client.send(
+ const result = await r2Client.send(
new DeleteObjectsCommand({
Bucket: env.R2_BUCKET_NAME,
Delete: {
Objects: keys.map(Key => ({ Key })),
Quiet: true
}
})
);
+
+ if (result.Errors && result.Errors.length > 0) {
+ throw new Error(`Failed to delete ${result.Errors.length} object(s) from R2`);
+ }
};📝 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.
| await r2Client.send( | |
| new DeleteObjectsCommand({ | |
| Bucket: env.R2_BUCKET_NAME, | |
| Delete: { | |
| Objects: keys.map(Key => ({ Key })), | |
| Quiet: true | |
| } | |
| }) | |
| ); | |
| const result = await r2Client.send( | |
| new DeleteObjectsCommand({ | |
| Bucket: env.R2_BUCKET_NAME, | |
| Delete: { | |
| Objects: keys.map(Key => ({ Key })), | |
| Quiet: true | |
| } | |
| }) | |
| ); | |
| if (result.Errors && result.Errors.length > 0) { | |
| throw new Error(`Failed to delete ${result.Errors.length} object(s) from R2`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/services/r2.service.ts`
around lines 48 - 56, The DeleteObjectsCommand response is ignored so partial
failures (returned in result.Errors when Quiet: true) are missed; change the
await r2Client.send(new DeleteObjectsCommand({...})) call to capture the result
(e.g., const result = await r2Client.send(...)) and inspect result.Errors (or
result.Deleted vs Errors) to detect per-key failures for the bucket
env.R2_BUCKET_NAME and the keys produced by keys.map(Key => ({ Key })); if any
Errors are present, surface them (throw an error or log and return failure)
identifying which Key(s) failed so callers can handle partial deletions.
| success: boolean; | ||
| message: string; | ||
| statusCode: StatusCode; | ||
| data?: T | null; |
There was a problem hiding this comment.
Inconsistent handling of null vs undefined in optional fields.
The data field is typed as T | null, suggesting null is a valid distinct value, but the spread check on line 38 uses !== undefined, which means explicit null values will appear in the JSON response as "data": null. The same issue exists for errors on line 39.
This creates inconsistent behavior:
data: undefined→ excluded from response ✓data: null→ included as"data": nullin response
If the intent is to exclude both null and undefined, use != null (loose equality catches both). If null should be serialized, remove | null from the type to clarify that optional means undefined-only.
Proposed fix to exclude both null and undefined
type ApiResponseParams<T> = {
success: boolean;
message: string;
statusCode: StatusCode;
- data?: T | null;
+ data?: T;
errors?: unknown;
};
export class ApiResponse<T = unknown> {
public readonly success: boolean;
public readonly message: string;
public readonly statusCode: StatusCode;
- public readonly data?: T | null;
+ public readonly data?: T;
public readonly errors?: unknown;
// ...
send(res: Response): Response {
return res.status(this.statusCode).json({
success: this.success,
message: this.message,
statusCode: this.statusCode,
- ...(this.data !== undefined && { data: this.data }),
- ...(this.errors !== undefined && { errors: this.errors })
+ ...(this.data != null && { data: this.data }),
+ ...(this.errors != null && { errors: this.errors })
});
}Also applies to: 16-16, 38-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/api-response.ts`
at line 8, The ApiResponse<T> interface declares data?: T | null (and errors?:
... | null) but the response builder only checks !== undefined when spreading,
so explicit nulls get serialized; change the spreading conditions in the
response builder (the code that currently checks data !== undefined and errors
!== undefined) to use != null so both null and undefined are excluded, or
alternatively remove | null from the ApiResponse<T> fields if you want null to
be serialized; adjust the checks for the data and errors fields in the response
construction logic accordingly (referencing ApiResponse<T>, the data and errors
fields, and the response builder/spread logic).
| target: "pino-pretty", | ||
| options: { | ||
| colorize: true, | ||
| translateTime: "yyyy-mm-dd HH:MM:ss", |
There was a problem hiding this comment.
Incorrect date format in pino-pretty configuration.
The translateTime format string uses mm for month and MM for minute, but pino-pretty (which uses the fecha library) expects the opposite:
MMfor month (capital M)mmfor minute (lowercase m)
The current format "yyyy-mm-dd HH:MM:ss" will display timestamps with the month and minute values swapped, e.g., showing "2024-30-15 14:05:45" when the actual time is May 15, 2024 at 14:30:45.
Proposed fix
options: {
colorize: true,
- translateTime: "yyyy-mm-dd HH:MM:ss",
+ translateTime: "yyyy-MM-dd HH:mm:ss",
ignore: "pid,hostname"
}📝 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.
| translateTime: "yyyy-mm-dd HH:MM:ss", | |
| options: { | |
| colorize: true, | |
| translateTime: "yyyy-MM-dd HH:mm:ss", | |
| ignore: "pid,hostname" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/templates/node/express/component/file-upload/cloudflare-r2/mvc/src/utils/logger.ts`
at line 12, The translateTime format in the pino-pretty configuration is using
the wrong month/minute tokens; update the translateTime string used in the
logger config (the property named translateTime in the pino/pino-pretty setup
inside logger.ts) from "yyyy-mm-dd HH:MM:ss" to the correct "yyyy-MM-dd
HH:mm:ss" so months use capital MM and minutes use lowercase mm; locate the
logger config/pinoPretty options in logger.ts and replace the format string
accordingly.
|
@pawanbhayde Add docs also for AWS S3 and Cloudflare R2. And add new registry in {
"slug": "file-upload-cloudflare-r2",
"title": "File Upload (Cloudflare R2)",
"description": "The File Upload component provides a standardized way to handle file uploads in Servercn using Cloudflare R2 as the storage provider.",
"type": "component",
"status": "stable",
"frameworks": ["express"],
"docs": "/docs/components/file-upload-cloudflare-r2",
"meta": {
"new": true
}
},
{
"slug": "file-upload-aws-s3",
"title": "File Upload (AWS S3)",
"description": "The File Upload component provides a standardized way to handle file uploads in Servercn using AWS S3 as the storage provider.",
"type": "component",
"status": "stable",
"frameworks": ["express"],
"docs": "/docs/components/file-upload-aws-s3",
"meta": {
"new": true
}
}Also, don’t forget to add yourself to the contributor section in the docs frontmatter, for example: |
|
@pawanbhayde Brother, Join Discord for discussion: https://discord.gg/xRz9ss3Ah |
updated |
providers
Add two new variants to the file-upload component registry:
AWS S3: uses @aws-sdk/client-s3 with PutObjectCommand/DeleteObjectCommand, generates a public URL in the format https://.s3..amazonaws.com/. Requires: AWS_REGION, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_S3_BUCKET_NAME.
Cloudflare R2: S3-compatible via @aws-sdk/client-s3 pointed at the R2 endpoint (https://.r2.cloudflarestorage.com). Public URL is built from a configurable R2_PUBLIC_URL env var. Requires: R2_ACCOUNT_ID, R2_ACCESS_KEY_ID, R2_SECRET_ACCESS_KEY, R2_BUCKET_NAME, R2_PUBLIC_URL.
Both providers scaffold multer (memoryStorage) → SDK upload
pipeline with
single-file, multi-file, and delete endpoints. Templates are
provided for
both MVC and Feature architectures. Env keys are
auto-injected into .env
and .env.example on install.
Summary by CodeRabbit
New Features
Chores