feat: add structured logger and global error handler (#37) - #45
Open
kodegreen70 wants to merge 1 commit into
Open
feat: add structured logger and global error handler (#37)#45kodegreen70 wants to merge 1 commit into
kodegreen70 wants to merge 1 commit into
Conversation
- Add pino logger (utils/logger.ts): JSON in prod, pretty-printed in dev - Add global errorHandler middleware (middleware/errorHandler.ts): logs method, path, IP and full stack; hides stack traces in production - Extract shared asyncHandler utility (utils/asyncHandler.ts); remove three identical per-file copies from auth, trades and wallet routes - Replace all console.error calls with structured logger.error
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
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.
PR: Centralized Error Handler & Structured Logger
Summary
Adds a structured pino logger (
utils/logger.ts) and a global Expresserror-handling middleware (
middleware/errorHandler.ts) as foundationalserver infrastructure. A shared
asyncHandlerutility replaces the threeidentical per-file copies that existed across the route modules.
Closes #37
Type of Change
feat— new featurefix— bug fixrefactor— code change with no behaviour changedocs— documentation onlychore— build, deps, configcontract— Soroban smart contract changeWhat Changed
server/src/utils/logger.tsAuthorizationheaders and any secret key fields.server/src/utils/asyncHandler.tsasyncHandlerwrapper — forwards async route errors tonext(err)so every handler is covered without individual try/catch.server/src/middleware/errorHandler.ts{ error: "Internal server error" }in production; full message + stack in development. Respects existing HTTP status codes on the error object.server/src/index.tsmorganand inlineconsole.errorerror handler. Imports and registerserrorHandleras the last middleware. Startup logging now useslogger.fatal/logger.info.server/src/routes/auth.tsasyncHandlercopy. Imports shared utility. Replaced threeconsole.errorcalls with structuredlogger.error.server/src/routes/trades.tsasyncHandlercopy. Imports shared utility.server/src/routes/wallet.tsasyncHandlercopy. Imports shared utility. Replacedconsole.errorwithlogger.error.server/package.jsonpino@9.3.2andpino-pretty@11.2.1as pinned dependencies.How It Works
Logger (
utils/logger.ts)NODE_ENV !== "production"): coloured, human-readable outputvia
pino-prettywith timestamps.log aggregators (Datadog, CloudWatch, etc.) without extra configuration.
req.headers.authorization,body.buyerSecretKey,body.sellerSecretKey,*.stellar_secret_key,*.encryptedSecretKey.Error Handler (
middleware/errorHandler.ts)Every unhandled error that reaches
next(err)is caught here:Errors with an explicit
statusCodeorstatusproperty (e.g. fromhttp-errors) retain their original HTTP status rather than defaulting to 500.asyncHandler (
utils/asyncHandler.ts)How to Test
Verify structured logging (dev):
# You should see coloured pino output in the terminal, not raw console lines curl http://localhost:3001/healthVerify error handler in development:
# Hit a route that throws — response includes message + stack curl http://localhost:3001/api/trades/not-a-real-uuidVerify production mode:
NODE_ENV=production npm run dev # Error responses must NOT include stack tracesTypeScript check:
npx tsc --noEmit # must exit 0 with no errorsChecklist
General
tsc --noEmit).env.exampleupdated if new env vars were added — no new env varsAPI changes
{ error: string }Notes for Reviewer
morganhas been removed fromindex.ts. Request-level logging can beadded back as a pino HTTP logger (
pino-http) in a follow-up if needed;pino-http integrates the request ID with the child logger automatically.
asyncHandlerinauth.ts,trades.ts, andwallet.tswerefunctionally identical — this PR consolidates them with no behaviour change.
pino-prettyis a runtime dependency (not devDependency) because the devserver runs via
ts-node-devand needs it available at runtime in dev mode.It is a no-op in production since the transport is only configured when
NODE_ENV !== "production".