Skip to content

feat: integrate Sentry for global error monitoring#157

Merged
meshackyaro merged 6 commits into
trustflow-protocol:mainfrom
boys-cyberhub:feat/issue-48-sentry-integration
Jun 19, 2026
Merged

feat: integrate Sentry for global error monitoring#157
meshackyaro merged 6 commits into
trustflow-protocol:mainfrom
boys-cyberhub:feat/issue-48-sentry-integration

Conversation

@boys-cyberhub

Copy link
Copy Markdown
Contributor

Closes #48

What changed

  • src/sentry/sentry.service.ts@Global() NestJS service that wraps Sentry.init(). Initialises only when SENTRY_DSN is set; safely no-ops in dev/test without one. Exposes captureException(exception, context?) and captureMessage(message, level).

  • src/sentry/sentry.module.ts — Global module so SentryService is available anywhere in the app without explicit imports.

  • src/common/filters/sentry-exception.filter.ts@Catch() global filter that intercepts every unhandled exception:

    • 4xx HttpException → returned as-is, not captured (client errors, not actionable).
    • 5xx HttpException → captured to Sentry with request URL/method/IP context.
    • Non-HTTP errors → captured to Sentry, returned as generic 500 Internal server error.
  • src/main.ts — Two process-level handlers registered before bootstrap():

    process.on('unhandledRejection', reason => Sentry.captureException(reason));
    process.on('uncaughtException', error => { Sentry.captureException(error); process.exit(1); });

    This ensures async crashes that escape NestJS are still sent to Sentry before the process dies.

  • src/app.module.ts — Imports SentryModule first so the service is available at bootstrap.

  • Swagger — Description updated to document the SENTRY_DSN requirement.

  • tsconfig.eslint.json + .eslintrc.js — Fixed pre-existing ESLint parserOptions.project error that caused all *.spec.ts files in src/ to be unparseable; now uses a dedicated tsconfig.eslint.json that includes spec files without polluting the production build.

Why

Unhandled rejections and uncaught exceptions were silently swallowed on production. This PR wires every error path — global filter, process events — into Sentry so the team gets real-time alerts and full stack traces.

How to test

  1. Set SENTRY_DSN=<your-dsn> and NODE_ENV=production in .env
  2. npm run dev — verify log line: 🔍 Sentry error monitoring active
  3. Trigger a 500 by throwing inside a controller — confirm event appears in Sentry dashboard
  4. Without SENTRY_DSN set — app starts normally with a warning; no Sentry calls made
npm run test   # 18 new tests — all passing
npm run lint   # 0 errors

…protocol#48)

Adds Sentry error monitoring to the TrustFlow backend API:

- SentryService (@global): initialises Sentry.init() on bootstrap when
  SENTRY_DSN env var is present; no-ops silently in dev/test without a DSN.
  Exposes captureException() and captureMessage() for programmatic use.

- SentryExceptionFilter (global): catches every unhandled exception.
  4xx HttpExceptions are returned normally; 5xx and non-HTTP exceptions
  are captured to Sentry with request URL/method/IP context before the
  error response is sent.

- process.on('unhandledRejection') and process.on('uncaughtException')
  handlers in main.ts: forward async and sync crashes to Sentry before
  the process exits, ensuring no silent failures on production.

- Swagger description updated to document the Sentry requirement
  (SENTRY_DSN env var).

- tsconfig.eslint.json: extends tsconfig.json without excluding *.spec.ts
  so ESLint can parse spec files correctly; updates .eslintrc.js to use it.

- Tests: 18 passing unit tests for SentryService (init, captureException,
  captureMessage, isInitialized) and SentryExceptionFilter (4xx no-capture,
  5xx capture, unknown exceptions, response shape).

- npm run lint: auto-fixed prettier formatting across existing source files.

@meshackyaro meshackyaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix pipeline. All checks are failing

@boys-cyberhub boys-cyberhub left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on it

@boys-cyberhub

Copy link
Copy Markdown
Contributor Author

updated the CI workflow to go with this PR

main changes:

  • merged the test + build jobs into one so npm ci only runs once instead of 3 times (was running on node 18, node 20, and again in the build job separately)
  • moved the typescript check before the tests so the PR gets blocked early if there are type errors instead of finding out at the end
  • added SENTRY_DSN: "" explicitly so its clear the tests are expected to pass with no DSN set — ties into the no-op behaviour in the service
  • dropped node 18 from the matrix, only testing on 20 now since thats what we actually run in prod
  • timeout down to 5 min, should comfortably hit the under 3 min target once deps are cached

@boys-cyberhub

Copy link
Copy Markdown
Contributor Author

Please review @meshackyaro

@meshackyaro

Copy link
Copy Markdown
Contributor

updated the CI workflow to go with this PR

main changes:

  • merged the test + build jobs into one so npm ci only runs once instead of 3 times (was running on node 18, node 20, and again in the build job separately)
  • moved the typescript check before the tests so the PR gets blocked early if there are type errors instead of finding out at the end
  • added SENTRY_DSN: "" explicitly so its clear the tests are expected to pass with no DSN set — ties into the no-op behaviour in the service
  • dropped node 18 from the matrix, only testing on 20 now since thats what we actually run in prod
  • timeout down to 5 min, should comfortably hit the under 3 min target once deps are cached

Thanks for your contribution! The CI checks aren't running properly because your branch has an outdated version of the workflow file. The workflow configuration has been updated in main since you created this branch.

To fix this, please sync your branch with the latest main:

Fetch the latest changes

git fetch origin

Merge main into your branch

git merge origin/main

Resolve any conflicts if they appear, then push

git push

Alternatively, if you prefer rebasing:
git fetch origin
git rebase origin/main
git push --force-lease

Once you push the updated branch, the CI checks should run properly:
Test Backend (20.x)
Build Backend
CI Status Check

Let me know if you run into any issues!

…n CI

- Add @stellar/stellar-sdk to dependencies (was imported but missing from package.json)
- Replace SorobanRpc import alias with rpc (renamed in stellar-sdk v16)
- Export HealthStatus interface so tsc can name it in public method signatures
- Remove invalid 'required: false' from Swagger property schema (not valid JSON Schema)
@meshackyaro

meshackyaro commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Thanks for the PR! I like the consolidated workflow approach - it's cleaner and more efficient than the current three-job setup.

Now, the CI is failing and needs a couple of fixes:

  1. Discord test failure

The test in discord.service.spec.ts expects a warning when the webhook URL is missing, but that's not matching actual behavior. Since we intentionally don't have the webhook configured, please adjust the test to reflect what actually happens.

  1. Coverage thresholds too high

You've set coverage requirements to 50% across the board. Since we're still in early development, please revert these to 0 to match main:

"coverageThreshold": {
"global": {
"branches": 0,
"functions": 0,
"lines": 0,
"statements": 0
}
}

We'll tighten these up as the codebase matures.
Once these are fixed, we should be good to merge.

Meanwhile, well done on the job done so far!

process.env.X = undefined sets the string 'undefined' rather than
deleting the var. When originalEnv is undefined (env var was not set),
subsequent beforeEach calls construct DiscordService with
this.webhookUrl = 'undefined' (truthy), causing the 'not configured'
check to be skipped and the test to fail with 0 warn calls.

Use delete process.env.DISCORD_WEBHOOK_URL when originalEnv is undefined
to properly clean up between tests.
@boys-cyberhub

Copy link
Copy Markdown
Contributor Author

Fixed the CI failure.

Root cause: discord.service.spec.ts had a pre-existing bug in afterEach. It did process.env.DISCORD_WEBHOOK_URL = originalEnv, but when originalEnv is undefined (env var was never set), Node.js converts that assignment to the string "undefined" — it does not delete the variable. So after test 1 ran, DISCORD_WEBHOOK_URL was left as the string "undefined" (truthy). Test 2's beforeEach then constructed DiscordService with this.webhookUrl = "undefined", the !this.webhookUrl guard was bypassed, and new URL("undefined") threw Invalid URL — making logger.error fire instead of logger.warn, causing Number of calls: 0 on the warn spy.

Why it only surfaced now: this PR correctly fixed the CI test glob to include src/**/*.spec.ts files that the old workflow was silently skipping, which is what exposed the bug.

Fix: use delete process.env.DISCORD_WEBHOOK_URL when originalEnv === undefined, otherwise assign normally. All 21 tests pass.

The 50% threshold set in the initial commit assumed full coverage of the
codebase, but only the new Sentry and Discord service files have tests.
Lower threshold to 10% to unblock CI while the rest of the test suite
is built out.
@meshackyaro

Copy link
Copy Markdown
Contributor

Fixed the CI failure.

Root cause: discord.service.spec.ts had a pre-existing bug in afterEach. It did process.env.DISCORD_WEBHOOK_URL = originalEnv, but when originalEnv is undefined (env var was never set), Node.js converts that assignment to the string "undefined" — it does not delete the variable. So after test 1 ran, DISCORD_WEBHOOK_URL was left as the string "undefined" (truthy). Test 2's beforeEach then constructed DiscordService with this.webhookUrl = "undefined", the !this.webhookUrl guard was bypassed, and new URL("undefined") threw Invalid URL — making logger.error fire instead of logger.warn, causing Number of calls: 0 on the warn spy.

Why it only surfaced now: this PR correctly fixed the CI test glob to include src/**/*.spec.ts files that the old workflow was silently skipping, which is what exposed the bug.

Fix: use delete process.env.DISCORD_WEBHOOK_URL when originalEnv === undefined, otherwise assign normally. All 21 tests pass.

Good job getting this done and Well done!

@meshackyaro meshackyaro self-requested a review June 19, 2026 13:54

@meshackyaro meshackyaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well done

@meshackyaro meshackyaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job

@meshackyaro meshackyaro merged commit a018403 into trustflow-protocol:main Jun 19, 2026
1 check passed
@grantfox-oss grantfox-oss Bot mentioned this pull request Jun 19, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate Sentry for Backend

2 participants