fix: production Docker build, health endpoint, and bug fixes#59
fix: production Docker build, health endpoint, and bug fixes#59ZiyadKakhandkikar wants to merge 3 commits into
Conversation
- Dockerfile now compiles TypeScript in a build stage and runs the compiled dist/src/main output in a slim production image, instead of running `npm start` (nest start, dev mode) inside the container. - Add GET /health so the Docker healthcheck has something to hit; previously there was no health route and the container was always marked unhealthy. - Remove a leftover debug console.log that printed a developer's name. - Fix updatePermission swallowing errors: the catch block built an error response but never returned it, so a failed update request would hang instead of getting a response. - Add .dockerignore so the build context skips node_modules, dist, coverage, and other local-only files. Co-authored-by: multica-agent <github@multica.ai>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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 several improvements, including a multi-stage Dockerfile build to optimize image size, the addition of a .dockerignore file, a new /health endpoint with associated unit tests, the removal of a debug log in the notification service, and a fix to return the error response in the role-permission mapping service. There are no review comments, and we have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
POST /notification-templates only ever wrote email/push/sms config rows — there was no way to create a NotificationActionTemplates row with type='inApp'. Since in-app sends resolve their content by looking up a published template with type='inApp' for the action (InAppService.resolveContent), every context+key send through POST /notification/send with an inApp block was guaranteed to fail with "In-app template not found or not published for resolved action". Adds an InAppNotificationDto (subject + body, same shape as the email DTO) to both CreateEventDto and UpdateEventDto, and wires it into createTemplate/ updateNotificationTemplate the same way email/push/sms already are. No entity/migration change needed — NotificationActionTemplates.type is a plain string column. Found while integrating this service into an external platform that needs email + in-app notifications from a single template. Co-authored-by: multica-agent <github@multica.ai>
|
Follow-up commit: added While seeding templates for the integration, discovered Added an |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| COPY . . | ||
| RUN npm run build | ||
|
|
||
| FROM node:20-slim AS production |
Found while standing up the service end-to-end for an external integration —
none of this surfaced before because local dev always ran against an
already-synced database.
- data-source.ts's migrations/entities globs were hardcoded to the .ts source
paths ('src/migrations/*.ts'). That works when run via ts-node
(npm run migration:*) but resolves to nothing when data-source.ts is
compiled to dist/data-source.js for production (nest build emits
dist/src/migrations/*.js, one directory deeper). migration:run against the
compiled file silently reported "No migrations are pending" — the tables
never got created. Switched to path.join(__dirname, 'src/migrations/*.{ts,js}'),
which resolves correctly in both the ts-node and compiled cases since
__dirname tracks whichever one is actually running.
- Since synchronize is false, migrations are the only thing that creates the
schema — nothing was running them on container start. Dockerfile now runs
`typeorm migration:run -d dist/data-source.js` before starting the app.
- CreateInAppNotification1764892800000 had a trailing comma before the
closing paren in its CREATE TABLE statement (left over from a removed
comment line) — invalid SQL, migration would fail as soon as it could
actually run.
- BackfillCoreTables1765200000000 created 4 uuid primary key columns
(NotificationActionTemplates.templateId, NotificationQueue.id,
RolePermission.rolePermissionId, notificationLogs.id) with no DEFAULT.
All 4 entities use @PrimaryGeneratedColumn('uuid') with no @BeforeInsert
hook, so they rely entirely on a DB-level default — every insert into any
of these tables (including creating a notification template) failed with
a NOT NULL violation on the primary key. Added DEFAULT uuid_generate_v4()
to each (the extension is already created automatically by TypeORM's
postgres driver on connect).
Verified: dropped the notifications DB, rebuilt the image, confirmed
migrations run automatically on a fresh container and all 8 tables + their
UUID defaults come up correctly.
Co-authored-by: multica-agent <github@multica.ai>
|
Follow-up commit: migrations never actually ran in production, and one had invalid SQL. While standing this service up end-to-end (postgres + rabbitmq + the compiled Docker image) I found the schema was never getting created:
Verified by dropping the |
|
2 participants
Summary
Fixes found while integrating this service into the Voice Note Analysis platform's docker-compose stack:
npm start(nest start), which is a dev-mode watcher, not a compiled production build. Switched to a two-stage build: stage 1 installs deps and runsnpm run buildto producedist/, stage 2 is a slim runtime image that only installs prod deps and runs the compileddist/src/main.jswithnode./healthendpoint. Docker/K8s healthchecks had nothing to hit and would mark the container permanently unhealthy. AddedGET /healthreturning{ status: "ok" }onAppController, plus a unit test.console.log(result.value, "Shubham")innotification.service.ts.returninupdatePermission. The catch block inrole-permission-mapping.service.tsbuilt an error response viaAPIResponse.error(...)but never returned it, so a failed/role-permission/updaterequest would hang without ever sending a response. Added the missingreturn..dockerignore. The build context was pulling innode_modules(200MB+),dist,coverage, and other local-only files. Added one.Test plan
npm run buildsucceeds and producesdist/src/main.js.npm test(app.controller.spec.ts) passes, including the new health check test.docker build .succeeds end-to-end with the new multi-stage Dockerfile.ECONNREFUSEDfor RabbitMQ/Postgres since none are running in the sandbox — confirming the compiled entrypoint (dist/src/main) is correct.