fix: add logging and genericize error responses in domains handler by SatishKumar620 · Pull Request #53 · OWASP-BLT/BLT-API

SatishKumar620 · 2026-03-14T17:09:38Z

Problem

In src/handlers/domains.py, all four error responses exposed raw
exception details to clients via str(e), and there was no logging
at all making production debugging impossible:

return error_response(str(e), status=503)
return error_response(f"Failed to fetch domain tags: {str(e)}", status=500)
return error_response(f"Failed to fetch domain: {str(e)}", status=500)
return error_response(f"Failed to fetch domains: {str(e)}", status=500)

Fix

Added logging import and logger instance
Full exception details logged server-side via logger.error()
Generic messages returned to clients
Consistent with pattern applied in organizations handler (PR fix: add error handling for DB queries in organizations handler #50)

Closes #52

Summary by CodeRabbit

Bug Fixes
- Standardized error responses for domain- and database-related failures to provide clearer, consistent messages to users.
- Added unobtrusive server-side logging for domain endpoints so errors are recorded for diagnostics while original HTTP status codes are preserved.
- Expanded error handling across domain operations to ensure consistent behavior and clearer feedback when failures occur.

- Add logging import and logger instance - Log full exception details server-side via logger.error() - Return generic messages to clients instead of raw str(e) - Consistent with pattern applied in organizations handler (PR OWASP-BLT#50) Closes OWASP-BLT#52

owasp-blt · 2026-03-14T17:09:44Z

📊 Monthly Leaderboard

Hi @SatishKumar620! Here's how you rank for March 2026:

Rank	User	Open PRs	PRs (merged)	Reviews	Comments	Total
11	`@DevalPrime`	5	20	0	3	211
12	`@SatishKumar620` ✨	17	4	2	65	197
13	`@aryanghai12`	13	13	3	19	196

Scoring this month (across OWASP-BLT org): Open PRs (+1 each), Merged PRs (+10), Closed (not merged) (−2), Reviews (+5; first two per PR in-month), Comments (+2, excludes CodeRabbit). Run /leaderboard on any issue or PR to see your rank!

owasp-blt · 2026-03-14T17:09:48Z

👋 Hi @SatishKumar620!

This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:

The PR author
coderabbitai
copilot

Once a valid peer review is submitted, this check will pass automatically. Thank you!

⚠️ Peer review enforcement is active.

coderabbitai · 2026-03-14T17:09:57Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: OWASP-BLT/coderabbit/.coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 38729cac-5c70-49c1-b9e5-cb2db18b12c1

📥 Commits

Reviewing files that changed from the base of the PR and between 549ce79 and 18eadec.

📒 Files selected for processing (1)

src/handlers/domains.py

🚧 Files skipped from review as they are similar to previous changes (1)

src/handlers/domains.py

Walkthrough

Introduced a module-level logger in src/handlers/domains.py and replaced responses that exposed raw exception strings with server-side logging plus generic error messages, preserving original HTTP status codes across four exception paths.

Changes

Cohort / File(s)	Summary
Error Handling & Logging `src/handlers/domains.py`	Added `logging` import and `logger = logging.getLogger(__name__)`. Updated four exception handlers to call `logger.error(..., exc_info=True)` and return generic client-facing error messages while keeping original status codes (503/500).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

quality: medium

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: adding logging and making error responses generic in the domains handler.
Linked Issues check	✅ Passed	All coding requirements from issue `#52` are met: logging added, exception details logged server-side with exc_info=True, and generic error messages returned to clients.
Out of Scope Changes check	✅ Passed	All changes are scoped to the domains handler and directly address the error handling and logging issues specified in issue `#52`.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📝 Coding Plan

Generate coding plan for human review comments

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

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 `@src/handlers/domains.py`:
- Around line 86-87: The logger call in the exception handler for fetching
domain tags (the logger.error call that currently logs f"Failed to fetch domain
tags: {str(e)}") should be updated to include full traceback by passing
exc_info=True; locate the exception block in the function that returns
error_response("Failed to fetch domain tags", status=500) and change the
logger.error invocation to include exc_info=True so the stack trace is recorded
alongside the error message.
- Around line 125-126: In the exception handler in src/handlers/domains.py (the
line calling logger.error(f"Failed to fetch domains: {str(e)}") before returning
error_response("Failed to fetch domains", status=500)), include the full
traceback by passing exc_info=True to logger.error so the log contains stack
trace information; keep the existing message and return behavior the same.
- Around line 98-99: The error log in src/handlers/domains.py currently calls
logger.error(f"Failed to fetch domain: {str(e)}") without the traceback; update
that logger.error invocation to include exc_info=True (e.g.,
logger.error("Failed to fetch domain", exc_info=True)) so the full traceback is
captured in the logs while keeping the existing error_response("Failed to fetch
domain", status=500) behavior intact.
- Around line 44-45: Replace the current single-line exception log so it
includes the full traceback and uses Ruff-recommended formatting: change the
logger.error call that currently formats the exception with str(e) (the
logger.error in this block) to include exc_info=True and use {e!s} for the
message, or use logger.exception(...) instead; keep the subsequent return
error_response("Database connection failed", status=503) intact. This ensures
the database connection error logs the full stack trace while preserving the
existing error response flow.

🪄 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: Repository: OWASP-BLT/coderabbit/.coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 06a45f94-42e6-4abf-b81b-65bd634dc218

📥 Commits

Reviewing files that changed from the base of the PR and between 7c6463b and 549ce79.

📒 Files selected for processing (1)

src/handlers/domains.py

src/handlers/domains.py

- Add exc_info=True to capture full stack traces for debugging - Replace str(e) with e!s per Ruff RUF010 recommendation

SatishKumar620 · 2026-03-14T17:22:05Z

@coderabbitai Please assess the PR's completeness and merge readiness within scope.

coderabbitai · 2026-03-14T17:23:21Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the PR diff and file contents
gh pr view 53 --repo OWASP-BLT/BLT-API --json title,body,state,reviews,checksums,statusCheckRollup 2>/dev/null || true
gh pr diff 53 --repo OWASP-BLT/BLT-API 2>/dev/null || true

Length of output: 2086

🏁 Script executed:

#!/bin/bash
# Fetch the full domains.py file and organizations handler for comparison
cat src/handlers/domains.py
echo "---ORG HANDLER---"
cat src/handlers/organizations.py 2>/dev/null || echo "File not found"

# Check if there are any open review comments on the PR
gh pr reviews 53 --repo OWASP-BLT/BLT-API 2>/dev/null || true

# Check CI status
gh pr checks 53 --repo OWASP-BLT/BLT-API 2>/dev/null || true

Length of output: 16960

@SatishKumar620 — Here's the merge-readiness assessment for PR #53:

✅ Completeness & Merge Readiness

Scope: Issue #52 — fully addressed. All 4 exception handlers in src/handlers/domains.py have been updated consistently:

Handler	Status Code	Logging	Generic Client Message
DB connection	503	✅ `exc_info=True`	`"Database connection failed"`
`/domains/{id}/tags`	500	✅ `exc_info=True`	`"Failed to fetch domain tags"`
`/domains/{id}`	500	✅ `exc_info=True`	`"Failed to fetch domain"`
`/domains` (list)	500	✅ `exc_info=True`	`"Failed to fetch domains"`

CI: Passing ✅
Open review threads: None ✅
Raw str(e) exposure to clients: Eliminated ✅
{e!s} (RUF010): Used correctly ✅
Original status codes preserved: ✅

⚠️ Minor Nit (Non-blocking)

Import ordering — import logging is inserted after from typing import Any, Dict, but both are stdlib. PEP 8 / isort convention places bare import X statements before from X import Y within the same group:

# Preferred
import logging
from typing import Any, Dict
from utils import ...

This is cosmetic and won't affect behaviour, but worth a one-line fix before merging if you want to stay isort-clean.

📝 Out-of-Scope Observation (Follow-up candidate)

Inspecting organizations.py, its DB connection handler still exposes raw exceptions:

# organizations.py — still unpatched
except Exception as e:
    return error_response(str(e), status=503)  # no logging, raw str(e)

The PR description references the organizations handler as the model pattern, but it hasn't been updated to match yet. Would you like me to open a follow-up issue to track this?

Verdict: Ready to merge after optionally fixing the import ordering nit.

Kunal241207

Nice improvement
One small suggestion: you might consider using logger.exception("message") instead of f-string logging with exc_info=True for slightly cleaner stacktrace handling and more consistent logging style.

SatishKumar620 · 2026-03-14T17:47:58Z

@Kunal241207 sure

arnavkirti

A couple of minor suggestions:

If the project uses a shared logger helper, it might be good to reuse that here instead of creating a module-level logger directly.
It could be worth adding tests to ensure we don’t regress and accidentally return raw exception messages again, and to verify the status codes and response schema.

Overall, looks good to me after addressing the minor points.

SatishKumar620 · 2026-03-14T18:21:19Z

@arnavkirti sure

owasp-blt bot added unresolved-conversations: 0 needs-peer-review PR needs peer review labels Mar 14, 2026

coderabbitai bot added the quality: medium label Mar 14, 2026

coderabbitai bot requested changes Mar 14, 2026

View reviewed changes

src/handlers/domains.py Outdated Show resolved Hide resolved

src/handlers/domains.py Outdated Show resolved Hide resolved

src/handlers/domains.py Outdated Show resolved Hide resolved

src/handlers/domains.py Outdated Show resolved Hide resolved

fix: add exc_info=True and use e!s in logger calls

18eadec

- Add exc_info=True to capture full stack traces for debugging - Replace str(e) with e!s per Ruff RUF010 recommendation

coderabbitai bot removed the quality: medium label Mar 14, 2026

coderabbitai bot approved these changes Mar 14, 2026

View reviewed changes

Kunal241207 reviewed Mar 14, 2026

View reviewed changes

arnavkirti reviewed Mar 14, 2026

View reviewed changes

DonnieBLT mentioned this pull request Mar 15, 2026

fix: sanitize error responses and add logging in domains handler #54

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add logging and genericize error responses in domains handler#53

fix: add logging and genericize error responses in domains handler#53
SatishKumar620 wants to merge 2 commits intoOWASP-BLT:mainfrom
SatishKumar620:fix/error-handling-domains

SatishKumar620 commented Mar 14, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

owasp-blt bot commented Mar 14, 2026

Uh oh!

owasp-blt bot commented Mar 14, 2026

Uh oh!

coderabbitai bot commented Mar 14, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SatishKumar620 commented Mar 14, 2026

Uh oh!

coderabbitai bot commented Mar 14, 2026

Uh oh!

Kunal241207 left a comment

Uh oh!

SatishKumar620 commented Mar 14, 2026

Uh oh!

arnavkirti left a comment

Uh oh!

SatishKumar620 commented Mar 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

SatishKumar620 commented Mar 14, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Summary by CodeRabbit

Uh oh!

owasp-blt bot commented Mar 14, 2026

📊 Monthly Leaderboard

Uh oh!

owasp-blt bot commented Mar 14, 2026

Uh oh!

coderabbitai bot commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SatishKumar620 commented Mar 14, 2026

Uh oh!

coderabbitai bot commented Mar 14, 2026

✅ Completeness & Merge Readiness

⚠️ Minor Nit (Non-blocking)

📝 Out-of-Scope Observation (Follow-up candidate)

Uh oh!

Kunal241207 left a comment

Choose a reason for hiding this comment

Uh oh!

SatishKumar620 commented Mar 14, 2026

Uh oh!

arnavkirti left a comment

Choose a reason for hiding this comment

Uh oh!

SatishKumar620 commented Mar 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SatishKumar620 commented Mar 14, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 14, 2026 •

edited

Loading