fix: handle HTTPException in get_item function

[davidemarcoli]

Pull Request Check List

Resolves: #issue-number-here

• Added tests for changed code.
• Updated documentation for changed code.

Description:

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling in item retrieval to ensure HTTP errors are properly returned with correct status codes, enhancing API reliability and response consistency.

[coderabbitai]

Walkthrough

Error handling in get_item was refined to re-raise HTTPException instances directly while preserving their HTTP semantics, rather than allowing them to be processed through generic exception handling paths. Existing database result cardinality validation remains intact.

Changes

Cohort / File(s)	Change Summary
Error handling refinement src/routers/secure/items.py	Modified get_item function to re-raise HTTPException without transformation, preventing HTTP exceptions from being swallowed by generic exception handling

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify HTTPException re-raising logic doesn't alter intended exception propagation
• Confirm existing "Multiple rows" exception handling remains unaffected

Poem

🐰 An exception caught, but not quite right—
Now HTTPExceptions see the light!
Re-raised with grace, no more delay,
Your error codes shall have their say! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is incomplete with placeholder text and no actual implementation details, making it impossible to understand the purpose, motivation, or scope of changes.	Replace placeholder 'Resolves: #issue-number-here' with the actual issue number, provide detailed description of the HTTPException handling changes, and explain the motivation.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: refining HTTPException handling in the get_item function to preserve HTTP semantics.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch bugfix/handle-http-exception-item-endpoint

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4c5ac3b and b400c9f.

📒 Files selected for processing (1)

• src/routers/secure/items.py (1 hunks)

🧰 Additional context used

🪛 Ruff (0.14.3)

src/routers/secure/items.py

404-404: Use raise without specifying exception name

Remove exception name

(TRY201)

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Fix correctly preserves HTTPException semantics, but consider two improvements.

The change successfully prevents HTTPException instances (like the 404 at line 401) from being swallowed and re-wrapped as 500 errors.

However:

1. Use bare raise instead of raise e to preserve the original traceback:

 if isinstance(e, HTTPException):
-    raise e
+    raise

2. Consider narrowing the try-except scope. The current try block (lines 394-415) wraps both the database query (line 395) and business logic (lines 396-401). This causes the HTTPException at line 401 to be caught unnecessarily. A cleaner structure would separate concerns:

try:
    item: MediaItem = session.execute(query).unique().scalar_one_or_none()
except Exception as e:
    # Handle only database exceptions here
    if "Multiple rows were found" in str(e):
        items = session.execute(query).unique().scalars().all()
        duplicate_ids = {item.id for item in items}
        logger.debug(f"Multiple items found with ID {id}: {duplicate_ids}")
        raise HTTPException(
            status_code=500,
            detail=f"Multiple items found with ID {id}: {duplicate_ids}",
        )
    logger.error(f"Error fetching item with ID {id}: {str(e)}")
    raise HTTPException(status_code=500, detail=str(e)) from e

# Business logic outside try block (no HTTPException re-raising needed)
if item:
    if extended:
        return item.to_extended_dict()
    return item.to_dict()
else:
    raise HTTPException(status_code=404, detail="Item not found")

Based on static analysis hints.

🧰 Tools

🪛 Ruff (0.14.3)

404-404: Use raise without specifying exception name

Remove exception name

(TRY201)

[dreulavelle]

If you dont need the original traceback then its fine, otherwise might want to swap to raise instead of raise e.

[Gaisberg]

Why not just except this?