chore: add health check and logging (#406)#460
Merged
ryzen-xp merged 2 commits intoMay 8, 2026
Conversation
|
@Listoncrypt Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughAdds structured request logging (nestjs-pino) with env-driven ChangesHealth + Logging Integration
Sequence Diagram(s)sequenceDiagram
participant Client
participant HealthController
participant HealthCheckService
participant HTTP as HTTP Server
participant DB as Database (TypeORM)
participant Stellar as Stellar RPC
Client->>HealthController: GET /health
activate HealthController
HealthController->>HealthCheckService: check()
activate HealthCheckService
par Health Checks
HealthCheckService->>HTTP: HTTP ping localhost:PORT
HTTP-->>HealthCheckService: 200 OK
HealthCheckService->>DB: DB ping (TypeORM)
DB-->>HealthCheckService: connected
HealthCheckService->>Stellar: POST JSON-RPC getHealth
Stellar-->>HealthCheckService: OK / Error
end
HealthCheckService->>HealthController: aggregated result
alt All healthy
HealthController-->>Client: 200 {status: "up", ...}
else Any unhealthy
HealthController-->>Client: 503 {status: "down", ...}
end
deactivate HealthCheckService
deactivate HealthController
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/app.module.ts`:
- Around line 22-27: Replace the static placeholders in the logging serializer
with real request properties: update the serializers.req function (the req
serializer) to accept the incoming request object and return method: req.method,
url: req.url (or req.originalUrl), and headers: req.headers (or a sanitized
subset) so logs reflect the actual HTTP method/path and header data for each
request; ensure you reference the serializers object and the req serializer when
making the change.
In `@backend/src/health/health.controller.ts`:
- Around line 17-20: The health check uses a brittle target by deriving port
from process.env.PORT into the local variable port and calling
this.http.pingCheck('http-server', `http://localhost:${port}`) which can
mismatch the actual server bind and probes the root path; change the
controller's HTTP check to target the same source the app actually binds to (or
a configured/explicit health endpoint) — e.g., replace the dynamic
localhost:port construction with a configurable base URL/health path or use an
internal liveness probe endpoint (referencing health.check and http.pingCheck)
so it hits a guaranteed 2xx path; also update the bootstrap/server startup to
read the same PORT env variable (aligning the listener and health probe source)
so they never diverge.
In `@backend/src/health/health.module.ts`:
- Around line 2-11: HealthModule fails to import HttpModule while
HealthController injects HttpHealthIndicator, causing DI/runtime errors; update
backend/src/health/health.module.ts to add HttpModule (from `@nestjs/axios`) into
the `@Module` imports alongside TerminusModule/ConfigModule/DatabaseModule and
ensure backend/package.json includes both "@nestjs/axios" and "axios"
dependencies so the HttpModule can be resolved at runtime.
In `@backend/src/health/stellar-health.indicator.ts`:
- Around line 15-33: Add a fetch timeout using AbortController (e.g., create a
controller, set a short setTimeout to controller.abort) around the current POST
to stellarRpcUrl to avoid hangs, then after response.ok parse the JSON body and
validate JSON-RPC-level health (ensure no "error" field and that "result"
indicates healthy per the RPC contract) before returning this.getStatus(key,
true); on any non-ok HTTP status, abort/timeout, JSON parse error, presence of
an RPC error, or an unhealthy RPC result, call this.getStatus(key, false, {
message: ... }) with a descriptive message including response.status or the
caught error message; update the existing fetch block that currently returns
this.getStatus(key, true) or throws new Error to perform these checks and use
controller.abort to enforce timeout.
🪄 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: 512f060e-9c40-4954-ae40-9ba376d5f67b
⛔ Files ignored due to path filters (50)
backend/dist/app.controller.d.tsis excluded by!**/dist/**backend/dist/app.controller.jsis excluded by!**/dist/**backend/dist/app.controller.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/app.module.d.tsis excluded by!**/dist/**backend/dist/app.module.jsis excluded by!**/dist/**backend/dist/app.module.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/app.service.d.tsis excluded by!**/dist/**backend/dist/app.service.jsis excluded by!**/dist/**backend/dist/app.service.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/config/config.module.d.tsis excluded by!**/dist/**backend/dist/config/config.module.jsis excluded by!**/dist/**backend/dist/config/config.module.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/config/config.service.d.tsis excluded by!**/dist/**backend/dist/config/config.service.jsis excluded by!**/dist/**backend/dist/config/config.service.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/data-source.d.tsis excluded by!**/dist/**backend/dist/database/data-source.jsis excluded by!**/dist/**backend/dist/database/data-source.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/database.module.d.tsis excluded by!**/dist/**backend/dist/database/database.module.jsis excluded by!**/dist/**backend/dist/database/database.module.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/entities/auto-pay-rule.entity.d.tsis excluded by!**/dist/**backend/dist/database/entities/auto-pay-rule.entity.jsis excluded by!**/dist/**backend/dist/database/entities/auto-pay-rule.entity.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/entities/index.d.tsis excluded by!**/dist/**backend/dist/database/entities/index.jsis excluded by!**/dist/**backend/dist/database/entities/index.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/entities/payment.entity.d.tsis excluded by!**/dist/**backend/dist/database/entities/payment.entity.jsis excluded by!**/dist/**backend/dist/database/entities/payment.entity.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/entities/username.entity.d.tsis excluded by!**/dist/**backend/dist/database/entities/username.entity.jsis excluded by!**/dist/**backend/dist/database/entities/username.entity.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/entities/vault.entity.d.tsis excluded by!**/dist/**backend/dist/database/entities/vault.entity.jsis excluded by!**/dist/**backend/dist/database/entities/vault.entity.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/database/migrations/1714000000000-InitialSchema.d.tsis excluded by!**/dist/**backend/dist/database/migrations/1714000000000-InitialSchema.jsis excluded by!**/dist/**backend/dist/database/migrations/1714000000000-InitialSchema.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/main.d.tsis excluded by!**/dist/**backend/dist/main.jsis excluded by!**/dist/**backend/dist/main.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/stellar/stellar.module.d.tsis excluded by!**/dist/**backend/dist/stellar/stellar.module.jsis excluded by!**/dist/**backend/dist/stellar/stellar.module.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/stellar/stellar.service.d.tsis excluded by!**/dist/**backend/dist/stellar/stellar.service.jsis excluded by!**/dist/**backend/dist/stellar/stellar.service.js.mapis excluded by!**/dist/**,!**/*.mapbackend/dist/tsconfig.build.tsbuildinfois excluded by!**/dist/**backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
backend/.env.examplebackend/package.jsonbackend/src/app.module.tsbackend/src/health/health.controller.tsbackend/src/health/health.module.tsbackend/src/health/stellar-health.indicator.tsbackend/src/main.ts
Comment on lines
+22
to
+27
| serializers: { | ||
| req: () => ({ | ||
| method: 'REQ', | ||
| url: 'URL', | ||
| headers: 'HEADERS', | ||
| }), |
There was a problem hiding this comment.
Request serializer currently logs placeholders instead of real request data.
The current req serializer emits static values, so logs won’t contain actual method/path for each request.
🔧 Proposed fix
serializers: {
- req: () => ({
- method: 'REQ',
- url: 'URL',
- headers: 'HEADERS',
+ req: (req) => ({
+ method: req.method,
+ path: req.url,
}),
},📝 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.
Suggested change
| serializers: { | |
| req: () => ({ | |
| method: 'REQ', | |
| url: 'URL', | |
| headers: 'HEADERS', | |
| }), | |
| serializers: { | |
| req: (req) => ({ | |
| method: req.method, | |
| path: req.url, | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/app.module.ts` around lines 22 - 27, Replace the static
placeholders in the logging serializer with real request properties: update the
serializers.req function (the req serializer) to accept the incoming request
object and return method: req.method, url: req.url (or req.originalUrl), and
headers: req.headers (or a sanitized subset) so logs reflect the actual HTTP
method/path and header data for each request; ensure you reference the
serializers object and the req serializer when making the change.
Comment on lines
+17
to
+20
| const port = process.env.PORT || '3000'; | ||
| return this.health.check([ | ||
| () => this.http.pingCheck('http-server', `http://localhost:${port}`), | ||
| () => this.stellar.pingCheck('stellar-rpc'), |
There was a problem hiding this comment.
HTTP server check target is brittle and can report false-down.
This check derives port from process.env.PORT while the app currently binds to a fixed port in bootstrap; when those differ, /health can fail even if the service is up. Also, probing the bare root path is fragile if / is not a 2xx endpoint.
🔧 Proposed fix (controller side)
- const port = process.env.PORT || '3000';
+ const port = Number(process.env.PORT ?? 3000);
return this.health.check([
- () => this.http.pingCheck('http-server', `http://localhost:${port}`),
+ () => this.http.pingCheck('http-server', `http://127.0.0.1:${port}/api/docs`),
() => this.stellar.pingCheck('stellar-rpc'),
() => this.db.pingCheck('database'),
]);Also align bootstrap to the same PORT source so the health probe and listener never diverge.
📝 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.
Suggested change
| const port = process.env.PORT || '3000'; | |
| return this.health.check([ | |
| () => this.http.pingCheck('http-server', `http://localhost:${port}`), | |
| () => this.stellar.pingCheck('stellar-rpc'), | |
| const port = Number(process.env.PORT ?? 3000); | |
| return this.health.check([ | |
| () => this.http.pingCheck('http-server', `http://127.0.0.1:${port}/api/docs`), | |
| () => this.stellar.pingCheck('stellar-rpc'), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/health/health.controller.ts` around lines 17 - 20, The health
check uses a brittle target by deriving port from process.env.PORT into the
local variable port and calling this.http.pingCheck('http-server',
`http://localhost:${port}`) which can mismatch the actual server bind and probes
the root path; change the controller's HTTP check to target the same source the
app actually binds to (or a configured/explicit health endpoint) — e.g., replace
the dynamic localhost:port construction with a configurable base URL/health path
or use an internal liveness probe endpoint (referencing health.check and
http.pingCheck) so it hits a guaranteed 2xx path; also update the
bootstrap/server startup to read the same PORT env variable (aligning the
listener and health probe source) so they never diverge.
Comment on lines
+2
to
+11
| import { TerminusModule } from '@nestjs/terminus'; | ||
| import { HealthController } from './health.controller'; | ||
| import { StellarHealthIndicator } from './stellar-health.indicator'; | ||
| import { ConfigModule } from '../config/config.module'; | ||
| import { DatabaseModule } from '../database/database.module'; | ||
|
|
||
| @Module({ | ||
| imports: [TerminusModule, ConfigModule, DatabaseModule], | ||
| controllers: [HealthController], | ||
| providers: [StellarHealthIndicator], |
There was a problem hiding this comment.
Import HttpModule for HttpHealthIndicator to avoid DI/runtime failures.
HealthController injects HttpHealthIndicator, but this module does not import HttpModule. Also, @nestjs/axios/axios are not present in the shown dependency list. This can fail health module initialization.
🔧 Proposed fix (module wiring)
+import { HttpModule } from '@nestjs/axios';
import { Module } from '@nestjs/common';
import { TerminusModule } from '@nestjs/terminus';
import { HealthController } from './health.controller';
import { StellarHealthIndicator } from './stellar-health.indicator';
import { ConfigModule } from '../config/config.module';
import { DatabaseModule } from '../database/database.module';
`@Module`({
- imports: [TerminusModule, ConfigModule, DatabaseModule],
+ imports: [TerminusModule, HttpModule, ConfigModule, DatabaseModule],
controllers: [HealthController],
providers: [StellarHealthIndicator],
})
export class HealthModule {}Run this to verify missing wiring/dependencies:
#!/bin/bash
set -euo pipefail
echo "== Dependency presence check =="
jq -r '.dependencies["@nestjs/axios"] // "MISSING: `@nestjs/axios`"' backend/package.json
jq -r '.dependencies["axios"] // "MISSING: axios"' backend/package.json
echo
echo "== Health module/controller wiring check =="
rg -n "HttpHealthIndicator|HttpModule|TerminusModule" backend/src/health/health.controller.ts backend/src/health/health.module.tsExpected: both deps are present and HttpModule is imported in HealthModule.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/health/health.module.ts` around lines 2 - 11, HealthModule fails
to import HttpModule while HealthController injects HttpHealthIndicator, causing
DI/runtime errors; update backend/src/health/health.module.ts to add HttpModule
(from `@nestjs/axios`) into the `@Module` imports alongside
TerminusModule/ConfigModule/DatabaseModule and ensure backend/package.json
includes both "@nestjs/axios" and "axios" dependencies so the HttpModule can be
resolved at runtime.
Contributor
Author
|
On it |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/.env.example`:
- Line 38: The KEEPER_SECRET_KEY entry currently has a space after the equals
sign which may be interpreted as a whitespace value; update the
KEEPER_SECRET_KEY line in backend/.env.example to use a truly empty assignment
(no space or characters after '=') so parsers treat it as empty rather than a
whitespace string.
- Around line 30-31: Remove the duplicate environment variable entries for
TYPEORM_SYNCHRONIZE and PORT in backend/.env.example by deleting the repeated
lines (the second occurrences of TYPEORM_SYNCHRONIZE and PORT) so each key is
defined only once; keep the original definitions (the first occurrences) and
ensure no other duplicate keys remain to avoid precedence ambiguity.
🪄 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: 34c66884-b8a9-4485-be33-2c6d72289ab0
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
backend/.env.examplebackend/package.jsonbackend/src/app.module.tsbackend/src/main.ts
✅ Files skipped from review due to trivial changes (3)
- backend/src/app.module.ts
- backend/src/main.ts
- backend/package.json
Comment on lines
+30
to
+31
| TYPEORM_SYNCHRONIZE=true | ||
| PORT=3000 |
There was a problem hiding this comment.
Remove duplicated TYPEORM_SYNCHRONIZE and PORT entries.
Line 30 and Line 31 duplicate keys already defined at Line 3 and Line 2. Keep only one definition per key to avoid precedence ambiguity and config drift.
Proposed patch
-TYPEORM_SYNCHRONIZE=true
-PORT=3000
-
# Logging Configuration
LOG_LEVEL=info📝 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.
Suggested change
| TYPEORM_SYNCHRONIZE=true | |
| PORT=3000 | |
| # Logging Configuration | |
| LOG_LEVEL=info |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 30-30: [DuplicatedKey] The TYPEORM_SYNCHRONIZE key is duplicated
(DuplicatedKey)
[warning] 31-31: [DuplicatedKey] The PORT key is duplicated
(DuplicatedKey)
[warning] 31-31: [UnorderedKey] The PORT key should go before the TYPEORM_SYNCHRONIZE key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/.env.example` around lines 30 - 31, Remove the duplicate environment
variable entries for TYPEORM_SYNCHRONIZE and PORT in backend/.env.example by
deleting the repeated lines (the second occurrences of TYPEORM_SYNCHRONIZE and
PORT) so each key is defined only once; keep the original definitions (the first
occurrences) and ensure no other duplicate keys remain to avoid precedence
ambiguity.
| # --- Keeper Service (Automation) --- | ||
| KEEPER_ENABLED=false # Toggle for background automation tasks | ||
| KEEPER_SECRET_KEY= # Private key used by the keeper to sign transactions No newline at end of file | ||
| KEEPER_SECRET_KEY= # Private key used by the keeper to sign transactions |
There was a problem hiding this comment.
Use a truly empty value for KEEPER_SECRET_KEY.
Line 38 has a space after =, which can become a whitespace value in some parsers/checks. Use an empty assignment instead.
Proposed patch
-KEEPER_SECRET_KEY= # Private key used by the keeper to sign transactions
+KEEPER_SECRET_KEY= # Private key used by the keeper to sign transactions🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 38-38: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 38-38: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/.env.example` at line 38, The KEEPER_SECRET_KEY entry currently has a
space after the equals sign which may be interpreted as a whitespace value;
update the KEEPER_SECRET_KEY line in backend/.env.example to use a truly empty
assignment (no space or characters after '=') so parsers treat it as empty
rather than a whitespace string.
Contributor
Author
|
Hello im waiting for my stella points. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #406
Summary by CodeRabbit
New Features
Chores