-
Notifications
You must be signed in to change notification settings - Fork 112
Fix: Add missing await keyword in authentication dependency and make … #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…Weaviate non-blocking
WalkthroughTwo bug fixes addressing async and error handling: token verification in dependencies now properly awaits the Supabase async call, and Weaviate connection failures during startup now log warnings and continue instead of fatally aborting, enabling degraded operation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (2)
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. Comment |
smokeyScraper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the await bug. Just revert the change in main.py and rest is good to go.
backend/main.py
Outdated
| except Exception as e: | ||
| logger.error(f"Failed to connect to Weaviate: {e}") | ||
| raise | ||
| logger.warning(f"Failed to connect to Weaviate during startup: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should let the backend start without all the services running before using the bot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done as requested sir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses a critical authentication bug and improves service reliability by adding graceful degradation for the Weaviate service.
Key changes:
- Fixed missing
awaitkeyword on async Supabase authentication call that was causing backend crashes - Modified Weaviate connection failure handling to allow backend startup even when Weaviate is unavailable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| backend/app/core/dependencies.py | Added missing await keyword to supabase.auth.get_user() async call, fixing AttributeError crashes when authenticated users accessed protected endpoints |
| backend/main.py | Changed Weaviate connection test from failing fast (raise) to graceful degradation (warning), allowing the backend to start successfully even when Weaviate service is unavailable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
backend/main.py
Outdated
| logger.warning(f"Failed to connect to Weaviate during startup: {e}") | ||
| logger.warning("Continuing without Weaviate - some features may not work") | ||
| # Don't raise - allow backend to start even if Weaviate is unavailable |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change to allow startup without Weaviate is good for availability, but creates an inconsistency with the health check endpoints. Currently, if Weaviate fails to connect during startup, the backend continues running (as intended), but the /health endpoint will return 503 (unhealthy) whenever called because it doesn't handle the case where Weaviate is unavailable.
Consider updating the health check endpoints to gracefully handle Weaviate being unavailable, similar to how the startup now handles it. The /health endpoint could return 200 with weaviate: "unavailable" instead of raising a 503 exception. This would be more consistent with the graceful degradation approach.
📝 Description
This PR fixes a critical authentication bug where the
awaitkeyword was missing from the Supabase async authentication call. When logged-in users attempted to access protected integration endpoints, the backend would crash withAttributeError: 'coroutine' object has no attribute 'user'.The bug was introduced in commit
cedaad3(October 16, 2025) by CodeRabbit during an automated refactoring pass, where theawaitkeyword was incorrectly removed.🔧 Changes Made
backend/app/core/dependencies.pyline 50): Added missingawaitkeyword tosupabase.auth.get_user(token)callbackend/main.pylines 68-72): Changed error handling fromraisetowarningto prevent backend crashes when Weaviate times out during startup📷 Screenshots or Visual Changes (if applicable)
N/A - Backend bug fix with no visual changes
✅ Checklist
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.