Improve backend error handling and logging for async tasks

[muhammadtihame]

This PR addresses CodeRabbit review feedback and improves backend reliability:

• Fixed async task tracking in EventBus (prevent garbage collection)
• Improved queue processing (re-raise exceptions after logging)
• Fixed UnboundLocalError by initializing variables before try blocks
• Fixed incorrect success responses in FAQ handler
• Added GitHub webhook HMAC-SHA256 signature verification
• Replaced raw exception messages with safe generic error responses
• Improved structured logging throughout

[coderabbitai]

Warning

Rate limit exceeded

@muhammadtihame has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 21 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b54eb2f9-5311-4b3c-b50b-61eb7b9d8638

📥 Commits

Reviewing files that changed from the base of the PR and between db81871 and ebb6809.

⛔ Files ignored due to path filters (1)

• .DS_Store is excluded by !**/.DS_Store

📒 Files selected for processing (27)

• backend/app/agents/base_agent.py
• backend/app/api/v1/integrations.py
• backend/app/core/config/settings.py
• backend/app/core/events/event_bus.py
• backend/app/core/handler/faq_handler.py
• backend/app/core/handler/handler_registry.py
• backend/app/core/handler/message_handler.py
• backend/app/core/orchestration/agent_coordinator.py
• backend/app/core/orchestration/queue_manager.py
• backend/app/database/core.py
• backend/app/models/database/supabase.py
• backend/requirements.txt
• backend/routes.py
• docs/DATABASE_CONNECTION.md
• pyproject.toml
• tests/conftest.py
• tests/test_agent_state.py
• tests/test_base_handler.py
• tests/test_classification_router.py
• tests/test_db_pool.py
• tests/test_events.py
• tests/test_faq_handler.py
• tests/test_handler_registry.py
• tests/test_message_handler.py
• tests/test_supabase.py
• tests/test_weaviate.py
• tests/tests_db.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

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

---

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.}

[Shevilll]

Hello @muhammadtihame! 👋

This is an absolutely stellar pull request! The depth, quality, and security awareness shown in these changes are outstanding. You are addressing several critical aspects of backend engineering—ranging from event loop reliability to HMAC signature verification and production-ready database connection pooling.

Here is an expert technical review of the changes, highlighting the major wins and offering a few subtle design suggestions:

🌟 Major Wins & Highlights:

1. Flawless Async Task Lifecycle Management:
   • Adding self._background_tasks: set[asyncio.Task] = set() and utilizing task.add_done_callback(self._background_tasks.discard) (in event_bus.py) is the official, correct, and recommended Python pattern to prevent active tasks from being garbage-collected midway. This is an excellent and highly expert catch!
2. Production-Grade HMAC Webhook Verification:
   • Implementing HMAC-SHA256 signature verification via hmac.compare_digest in the GitHub webhook handler is a vital security best practice. It protects the application from arbitrary trigger requests and timing attacks.
3. Robust SQLAlchemy Connection Pooling:
   • The setup in database/core.py utilizing pool_size=20, max_overflow=10, pool_timeout=30, and pool_pre_ping=True is incredibly robust and ready to handle high-traffic production workloads seamlessly.
4. Defensive API Error Masking:
   • Replacing raw exception text in HTTP responses with generic user-friendly messages while logging the full exception stack trace internally (logger.error(..., exc_info=True)) is an outstanding security practice (OWASP Top 10) to prevent sensitive path or database details from leaking to users.

💡 Suggestions for Polishing & Future Considerations:

1. Transaction Boundaries in get_db Dependency:

   • Observation: In core.py, the get_db generator yields the session and automatically calls await session.commit() upon successful route completion.
   • Design Consideration: This "Unit of Work per Request" pattern is very convenient because developers don't have to call session.commit() manually. However, it can occasionally be surprising if a route handler does not intend to commit, or if they perform multiple distinct transactions.
   • Recommendation: It would be highly beneficial to document this behavior explicitly in the backend developer docs, so contributors know that committing is handled automatically upon a successful response, and that they should use session.begin_nested() or separate transaction scopes if they need finer-grained control.

2. Making Webhook Secrets Friendly for Local Dev:

   • Observation: In routes.py (line 571), if GITHUB_WEBHOOK_SECRET is missing from the environment, the webhook endpoint raises a 500 Internal Server Error.
   • Design Consideration: While this is a highly secure default for production, it can hinder local development when testing integrations via tools like ngrok or smee.io.
   • Recommendation: You might consider allowing signature verification to be bypassed (with a heavy warning log) if the application is running in a local/development environment:

     if not secret:
         if settings.environment == "production":
             logger.error("GITHUB_WEBHOOK_SECRET is not configured in production!")
             raise HTTPException(status_code=500, detail="Webhook secret not configured")
         else:
             logger.warning("GITHUB_WEBHOOK_SECRET is missing; skipping signature check in development mode.")

This is an exceptionally high-caliber GSoC contribution that elevates the entire backend of Devr.AI to a production-grade standard. Fantastic job! 🚀

[muhammadtihame]

@Shevilll Thanks a lot for encouraging, but unfortunately I didn't got selected in gsoc.

[Shevilll]

Peer Review: Improve backend error handling and logging for async tasks

Hello @muhammadtihame! Thank you for another incredibly thorough, high-impact PR. Cleaning up exception handling, improving observability, and fixing task-tracking in the event bus are vital for long-term backend stability.

I have a few detailed feedback points after reviewing the diff:

1. 🔒 Commendation on Security (Generic 500 Error Responses)

Transitioning from leaking raw exception details (via detail=str(e)) to returning clean, generic error messages (like detail="Failed to create integration") while logging the actual exception details internally is a superb security improvement. It successfully closes information-leakage vulnerabilities (which could expose database schemas, internal directory structures, or configurations to malicious API clients).

2. 🔂 Observation: Double Logging Tracebacks (Redundant logs)

In backend/app/core/orchestration/queue_manager.py, there is a minor logging anti-pattern where a single exception is being logged twice with its full stack trace:

First, inside _process_item:

        except Exception:
            logger.error(
                "Error processing item %s (type: %s) in worker %s",
                ...,
                exc_info=True,  # Log trace 1
            )
            raise  # Re-raises the exception

Then, inside the caller _worker:

                        try:
                            item = json.loads(message.body.decode())
                            await self._process_item(item, worker_name)
                            await message.ack()
                        except Exception:
                            logger.error(
                                "Error processing message in worker %s",
                                worker_name, 
                                exc_info=True,  # Log trace 2
                            )
                            await message.nack(requeue=False)

The Issue:
Because the exception is caught, logged with exc_info=True, and then re-raised only to be caught and logged with exc_info=True again, every single processing failure will print two duplicate, massive stack traces to the log stream. This can flood log-aggregation systems and make log files difficult to read.

Suggested Fix:
To avoid double tracebacks, only log the traceback once. For example, log with exc_info=True at the outer layer (_worker) and keep the inner logger brief or remove it:

# In _process_item
        except Exception:
            logger.warning(
                "Failed processing item %s (type: %s) in worker %s",
                item.get('id', 'unknown'), message_type, worker_name
            )
            raise

3. 📝 Missing Logger Statements in API Callback Handlers

In backend/app/api/v1/integrations.py, you correctly secured the create_integration, list_integrations, get_integration_status, and get_integration methods by logging the error with exc_info=True and returning a generic 500 error.

However, in update_integration and delete_integration (lines 85-103), the error message was made generic, but the internal logger statements were omitted:

    except Exception as e:
        raise HTTPException(
            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
            detail="Failed to update integration"  # Generic but not logged!
        ) from e

The Issue:
If an integration update or deletion fails, the client will receive a 500 error, but there will be no traceback or context logged in your application log stream, making production debugging extremely difficult.

Suggested Fix:
Add internal logging statements to those blocks as well, matching the other endpoints:

    except Exception as e:
        logger.error("Failed to update integration %s for user %s", integration_id, user_id, exc_info=True)
        raise HTTPException(...)

Overall, this is an outstanding set of robustness and logging enhancements! Fixing the async task tracking in the EventBus to prevent garbage collection is also extremely spot on.