Fix: Implement Robust Database Connectivity with IPv6 Handling and Graceful Error Recovery #242
Conversation
…aceful error recovery
WalkthroughIntroduces a comprehensive database connection system for PostgreSQL/Supabase with centralized configuration, retry logic with exponential backoff, IPv6 resilience checks, degraded-mode support via REST API fallback, structured error handling, health check endpoints, and enhanced logging across database, application, and AI routes. Changes
Sequence Diagram(s)sequenceDiagram
participant App as FastAPI App
participant Init as initialize_database()
participant Retry as create_engine_with_retry()
participant IPv6 as IPv6 Check
participant Engine as SQLAlchemy<br/>AsyncEngine
participant Test as test_connection()
participant DB as Database
participant REST as Supabase<br/>REST API
App->>Init: startup event
Init->>Retry: create engine with config
Retry->>IPv6: check IPv6 connectivity
alt IPv6 issues detected
IPv6-->>Retry: warning + fallback hints
end
loop retry with exponential backoff
Retry->>Engine: create AsyncEngine
Engine->>Test: validate connection
Test->>DB: test query
alt connection success
DB-->>Test: success
Test-->>Retry: return True
Retry-->>Init: return engine
else connection timeout/failure
DB-->>Test: error
Test-->>Retry: return False
Retry->>Retry: wait + retry
end
end
alt all retries exhausted
Retry-->>Init: return None
Init->>REST: check Supabase REST fallback
alt REST available
Init->>App: degraded mode enabled
Note over App: session yielding None<br/>via get_db_optional()
else REST unavailable
Init->>App: no DB or fallback
end
else engine created
Init->>App: database initialized
end
App->>App: seed data, validate schema
App-->>App: startup complete
sequenceDiagram
participant Route as API Route
participant get_db as get_db() /<br/>get_db_optional()
participant Engine as AsyncEngine
participant Session as AsyncSession
participant DB as Database
alt Mandatory Connection (get_db)
Route->>get_db: request session
get_db->>Engine: acquire from pool
Engine->>Session: create session
Session->>DB: execute operations
alt success
DB-->>Session: result
Session-->>Route: yield session
Route->>get_db: return
get_db->>Session: commit & close
else error
Route->>get_db: exception
get_db->>Session: rollback & close
get_db-->>Route: raise
end
else Degraded Mode (get_db_optional)
Route->>get_db: request optional session
alt database connected
get_db->>Session: yield session (same as above)
else database unavailable
get_db-->>Route: yield None
Note over Route: handle gracefully<br/>fallback to cache/mock
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (15)
Backend/RELEASE_NOTES.md (1)
39-83: Add language specifiers to fenced code blocks.The markdown linter flags several code blocks without language specifiers. For output/status examples, use
textorplaintextto satisfy linting and improve rendering.Example fix for one block:
-``` +```text ❌ Server crashes with "getaddrinfo failed" errorBackend/.env.example (1)
8-12: Consider uppercase naming for consistency.Database credential keys (
user,password,host,port,dbname) use lowercase while other environment variables use uppercase (e.g.,DB_POOL_SIZE,SUPABASE_URL). This inconsistency may cause confusion.If changing to uppercase, you'll also need to update
Backend/app/config.pylines 17-21 to match.Backend/app/db/seed.py (1)
66-68: Uselogger.exceptionto include stack traces.When logging errors in exception handlers,
logger.exception()automatically includes the stack trace, which aids debugging.except Exception as e: - logger.error(f"Failed to seed user {user_data.get('email')}: {e}") + logger.exception(f"Failed to seed user {user_data.get('email')}: {e}") continueBackend/DATABASE_SETUP.md (2)
79-83: Add language specifiers to error message code blocks.For consistency and proper rendering, add
textorplaintextas the language specifier for error message examples throughout this document (lines 79, 124, 135, 154, 231).-``` +```text ❌ Connection attempt failed: getaddrinfo failed
184-186: Add language specifier to endpoint examples.Health check endpoint blocks (lines 184, 202) should specify a language for proper rendering.
-``` +```http GET /Backend/app/config.py (1)
24-32: Add error handling for environment variable type conversions.If
DB_POOL_SIZEor similar env vars contain non-numeric values,int()will raiseValueErrorand crash on import. Consider adding fallback handling.- self.pool_size = int(os.getenv("DB_POOL_SIZE", "5")) + try: + self.pool_size = int(os.getenv("DB_POOL_SIZE", "5")) + except ValueError: + logger.warning("Invalid DB_POOL_SIZE, using default: 5") + self.pool_size = 5Apply similar pattern to other numeric conversions (
max_overflow,pool_timeout,pool_recycle,max_retries,retry_delay,connection_timeout).Backend/app/routes/ai.py (3)
57-61: Replace print statements with logger calls.These debug print statements should use
logger.debug()for consistency with the rest of the codebase and to allow log level control.- print("Gemini raw response:", resp.text) + logger.debug("Gemini raw response: %s", resp.text) data = resp.json() - print("Gemini parsed JSON:", data) + logger.debug("Gemini parsed JSON: %s", data) text = data['candidates'][0]['content']['parts'][0]['text'] - print("Gemini text to parse as JSON:", text) + logger.debug("Gemini text to parse as JSON: %s", text)
137-140: Chain exceptions and sanitize error details.Use
raise ... from efor proper exception chaining. Also, exposing raw exception strings in API responses may leak internal details.- raise HTTPException( - status_code=503, - detail=f"Failed to fetch trending niches: {str(e)}" - ) + raise HTTPException( + status_code=503, + detail="Failed to fetch trending niches. Please try again later." + ) from eApply similar pattern at line 172-175:
- raise HTTPException( - status_code=500, - detail=f"Internal server error: {str(e)}" - ) + raise HTTPException( + status_code=500, + detail="Internal server error" + ) from e
109-110: Uselogger.exceptionfor error logging in except blocks.
logger.exception()automatically includes the stack trace, improving debuggability.except Exception as e: - logger.error(f"Failed to fetch cached data: {e}") + logger.exception("Failed to fetch cached data")Apply to other error handlers at lines 128, 135, 151, 171.
Backend/app/main.py (3)
75-86: Consider multi-line log for SQL script.Logging SQL line-by-line creates many log entries. A single multi-line log would be cleaner.
- logger.info("") - logger.info("To create the trending_niches table, run:") - logger.info("") - logger.info(" CREATE TABLE trending_niches (") - logger.info(" id SERIAL PRIMARY KEY,") - logger.info(" name VARCHAR(255) NOT NULL,") - logger.info(" insight TEXT,") - logger.info(" global_activity INTEGER DEFAULT 1,") - logger.info(" fetched_at DATE NOT NULL,") - logger.info(" created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP") - logger.info(" );") - logger.info("") + logger.info( + "To create the trending_niches table, run:\n\n" + " CREATE TABLE trending_niches (\n" + " id SERIAL PRIMARY KEY,\n" + " name VARCHAR(255) NOT NULL,\n" + " insight TEXT,\n" + " global_activity INTEGER DEFAULT 1,\n" + " fetched_at DATE NOT NULL,\n" + " created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP\n" + " );\n" + )
151-156: Use CORS_ORIGINS from environment variable.CORS origins are hardcoded, but
.env.exampledefinesCORS_ORIGINSfor configuration. This should be read from the environment for flexibility.+# Parse CORS origins from environment +cors_origins_str = os.getenv("CORS_ORIGINS", "http://localhost:5173,http://localhost:3000") +cors_origins = [origin.strip() for origin in cors_origins_str.split(",")] + app.add_middleware( CORSMiddleware, - allow_origins=[ - "http://localhost:5173", - "http://localhost:3000", - "http://127.0.0.1:5173", - "http://127.0.0.1:3000" - ], + allow_origins=cors_origins, allow_credentials=True, allow_methods=["*"], allow_headers=["*"], )
197-201: Complete or remove the timestamp placeholder.The
timestampfield is set toNonewith a comment suggesting it could be added. Either implement it or remove the field to avoid confusion.+from datetime import datetime, timezone + @app.get("/health") async def health_check(): """Detailed health check endpoint""" status = get_connection_status() return { "status": "healthy" if status["connected"] else "degraded", "database": status, - "timestamp": None # Could add timestamp if needed + "timestamp": datetime.now(timezone.utc).isoformat() }Would you like me to implement this?
Backend/app/db/db.py (3)
56-64: Uselogger.exceptionto include the traceback.The error handler should use
logger.exceptioninstead oflogger.errorto automatically include the exception traceback, which aids debugging.Apply this diff:
except Exception as e: - logger.error(f"Connection test failed: {e}") + logger.exception("Connection test failed") return False
103-121: Consider makingcommand_timeoutconfigurable.The
command_timeoutis hardcoded to 60 seconds (line 106). For consistency with other timeout parameters, consider adding this todb_config.In
Backend/app/config.py, add:command_timeout: int = 60 # or appropriate field in DatabaseConfigThen use it here:
connect_args = { "ssl": db_config.ssl_mode if db_config.ssl_mode != "disable" else None, "timeout": db_config.connection_timeout, - "command_timeout": 60, + "command_timeout": db_config.command_timeout, }
131-136: Uselogger.exceptionfor the initial error to capture traceback.Line 135 should use
logger.exceptioninstead oflogger.errorto automatically include the exception traceback, which is valuable for debugging connection failures.Apply this diff:
except (SQLAlchemyError, OperationalError, OSError, socket.gaierror) as e: error_msg = str(e) db_connection_error = error_msg - logger.error(f"❌ Connection attempt {attempt} failed: {error_msg}") + logger.exception(f"❌ Connection attempt {attempt} failed")Note: The subsequent
logger.errorcalls (lines 139-154) are appropriate as they provide structured guidance messages, not exception logging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Backend/.env.example(1 hunks)Backend/DATABASE_SETUP.md(1 hunks)Backend/RELEASE_NOTES.md(1 hunks)Backend/app/config.py(1 hunks)Backend/app/db/db.py(1 hunks)Backend/app/db/seed.py(2 hunks)Backend/app/main.py(3 hunks)Backend/app/routes/ai.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T12:24:11.598Z
Learnt from: Saahi30
Repo: AOSSIE-Org/InPactAI PR: 145
File: backend/SQL:98-112
Timestamp: 2025-11-08T12:24:11.598Z
Learning: In the InPactAI repository, the backend/SQL file is a documentation reference that points to schema-reference.md and is not meant to be executed directly. Table ordering and FK constraint issues should not be flagged for this file.
Applied to files:
Backend/DATABASE_SETUP.md
🧬 Code graph analysis (4)
Backend/app/db/seed.py (2)
Backend/app/db/db.py (1)
is_database_connected(252-254)Backend/app/models/models.py (1)
User(25-53)
Backend/app/db/db.py (1)
Backend/app/config.py (4)
is_configured(49-51)get_missing_vars(57-68)get_database_url(43-47)has_supabase_fallback(53-55)
Backend/app/main.py (2)
Backend/app/db/db.py (4)
initialize_database(178-200)close_database(203-212)is_database_connected(252-254)get_connection_status(257-264)Backend/app/db/seed.py (1)
seed_db(9-75)
Backend/app/routes/ai.py (1)
Frontend/src/utils/supabase.tsx (1)
supabase(11-11)
🪛 dotenv-linter (4.0.0)
Backend/.env.example
[warning] 8-8: [LowercaseKey] The user key should be in uppercase
(LowercaseKey)
[warning] 9-9: [LowercaseKey] The password key should be in uppercase
(LowercaseKey)
[warning] 9-9: [UnorderedKey] The password key should go before the user key
(UnorderedKey)
[warning] 10-10: [LowercaseKey] The host key should be in uppercase
(LowercaseKey)
[warning] 10-10: [UnorderedKey] The host key should go before the password key
(UnorderedKey)
[warning] 11-11: [LowercaseKey] The port key should be in uppercase
(LowercaseKey)
[warning] 11-11: [UnorderedKey] The port key should go before the user key
(UnorderedKey)
[warning] 12-12: [LowercaseKey] The dbname key should be in uppercase
(LowercaseKey)
[warning] 12-12: [UnorderedKey] The dbname key should go before the host key
(UnorderedKey)
[warning] 32-32: [UnorderedKey] The SUPABASE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 54-54: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 55-55: [UnorderedKey] The DB_MAX_OVERFLOW key should go before the DB_POOL_SIZE key
(UnorderedKey)
[warning] 55-55: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 56-56: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 57-57: [UnorderedKey] The DB_POOL_RECYCLE key should go before the DB_POOL_SIZE key
(UnorderedKey)
[warning] 57-57: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 60-60: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 61-61: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 62-62: [UnorderedKey] The DB_CONNECTION_TIMEOUT key should go before the DB_MAX_RETRIES key
(UnorderedKey)
[warning] 62-62: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 65-65: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 66-66: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 69-69: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🪛 LanguageTool
Backend/RELEASE_NOTES.md
[style] ~329-~329: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...s. --- Release Date: December 14, 2025 Version: 2.0.0 Status: ✅ St...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.18.1)
Backend/DATABASE_SETUP.md
79-79: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
135-135: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
184-184: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
202-202: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
231-231: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Backend/RELEASE_NOTES.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
226-226: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.8)
Backend/app/db/seed.py
66-66: Do not catch blind exception: Exception
(BLE001)
67-67: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
73-73: Do not catch blind exception: Exception
(BLE001)
74-74: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
Backend/app/db/db.py
46-46: Consider moving this statement to an else block
(TRY300)
47-47: Do not catch blind exception: Exception
(BLE001)
50-50: Consider moving this statement to an else block
(TRY300)
51-51: Do not catch blind exception: Exception
(BLE001)
61-61: Consider moving this statement to an else block
(TRY300)
62-62: Do not catch blind exception: Exception
(BLE001)
63-63: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
129-129: Avoid specifying long messages outside the exception class
(TRY003)
135-135: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
139-139: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
140-140: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
141-141: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
142-142: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
143-143: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
144-144: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
145-145: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
146-146: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
147-147: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
148-148: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
149-149: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
150-150: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
151-151: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
152-152: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
153-153: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
154-154: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
161-161: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
169-169: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
170-170: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
171-171: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
218-220: Avoid specifying long messages outside the exception class
(TRY003)
Backend/app/main.py
48-48: Consider moving this statement to an else block
(TRY300)
50-50: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
52-52: Do not catch blind exception: Exception
(BLE001)
53-53: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
67-67: Local variable inspector is assigned to but never used
Remove assignment to unused variable inspector
(F841)
90-90: Do not catch blind exception: Exception
(BLE001)
96-96: Unused function argument: app
(ARG001)
116-116: Do not catch blind exception: Exception
(BLE001)
123-123: Do not catch blind exception: Exception
(BLE001)
124-124: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
184-184: Do not catch blind exception: Exception
(BLE001)
185-185: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
205-205: Unused function argument: request
(ARG001)
Backend/app/routes/ai.py
32-32: Do not catch blind exception: Exception
(BLE001)
33-33: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
109-109: Do not catch blind exception: Exception
(BLE001)
110-110: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
112-115: Abstract raise to an inner function
(TRY301)
127-127: Do not catch blind exception: Exception
(BLE001)
128-128: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
134-134: Do not catch blind exception: Exception
(BLE001)
135-135: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
137-140: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
139-139: Use explicit conversion flag
Replace with conversion flag
(RUF010)
142-142: Consider moving this statement to an else block
(TRY300)
146-146: Do not catch blind exception: Exception
(BLE001)
151-151: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
152-169: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
171-171: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
172-175: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
174-174: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (10)
Backend/app/db/seed.py (1)
70-74: Commit placement may cause partial seeding without persistence.The
commit()at line 71 is inside thetryblock but after the loop. If an exception occurs during the loop (after adding some users), those users won't be committed. If the intent is atomic all-or-nothing seeding, this is correct. However, if you want to persist successfully added users even on partial failure, move the commit inside the per-user try block.Also, use
logger.exceptionfor the outer error handler.- except Exception as e: - logger.error(f"❌ Database seeding failed: {e}") + except Exception as e: + logger.exception(f"❌ Database seeding failed: {e}")Backend/app/main.py (1)
204-228: Good implementation of global exception handler.The global exception handler effectively prevents server crashes and provides appropriate status codes (503 for DB issues, 500 for others). The conditional debug detail exposure is a good security practice.
Backend/app/db/db.py (8)
1-13: LGTM!Imports are appropriate and well-organized for the database connectivity module.
25-27: LGTM!Custom exception class is appropriately defined for database connection errors.
178-201: LGTM!The initialization function properly handles both successful connection and degraded mode scenarios, with appropriate state management.
203-212: LGTM!Database cleanup is properly implemented with engine disposal and state reset.
215-231: LGTM!The session dependency correctly enforces database connectivity and implements proper transaction management with commit/rollback/close patterns.
Note: Static analysis flags the long error message (TRY003), but providing detailed context in this exception is appropriate for debugging.
233-250: LGTM!The optional session dependency is correctly implemented with
@asynccontextmanagerand enables graceful degradation by yieldingNonewhen the database is unavailable.
252-264: LGTM!Status helper functions are clean and provide appropriate connection state information for health checks.
15-22: The global state variables are initialized once during application startup within FastAPI'slifespancontext manager, which is guaranteed to execute only once. No concurrent access risk exists.Likely an incorrect or invalid review comment.
| def get_database_url(self) -> Optional[str]: | ||
| """Construct database URL from environment variables""" | ||
| if not all([self.user, self.password, self.host, self.dbname]): | ||
| return None | ||
| return f"postgresql+asyncpg://{self.user}:{self.password}@{self.host}:{self.port}/{self.dbname}" |
There was a problem hiding this comment.
URL-encode password to handle special characters.
If the password contains special characters like @, #, /, or %, the database URL will be malformed and connections will fail silently or with confusing errors.
+from urllib.parse import quote_plus
+
def get_database_url(self) -> Optional[str]:
"""Construct database URL from environment variables"""
if not all([self.user, self.password, self.host, self.dbname]):
return None
- return f"postgresql+asyncpg://{self.user}:{self.password}@{self.host}:{self.port}/{self.dbname}"
+ encoded_password = quote_plus(self.password)
+ return f"postgresql+asyncpg://{self.user}:{encoded_password}@{self.host}:{self.port}/{self.dbname}"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Backend/app/config.py around lines 43 to 47, the constructed DB URL uses the
raw password which breaks when it contains special characters; URL-encode the
password (e.g., with urllib.parse.quote_plus) before embedding it into the
connection string so characters like @, #, /, % are escaped; update the code to
import and use urllib.parse.quote_plus (or equivalent) to encode the password
(and optionally username) when building the f-string, leaving the rest of the
URL construction unchanged.
| def check_ipv6_connectivity(host: str) -> bool: | ||
| """Check if the host resolves to IPv6 and if we can connect to it""" | ||
| try: | ||
| addr_info = socket.getaddrinfo(host, None) | ||
| has_ipv6 = any(addr[0] == socket.AF_INET6 for addr in addr_info) | ||
|
|
||
| if has_ipv6: | ||
| logger.info(f"🔍 Host {host} resolves to IPv6 addresses") | ||
| # Try to create a test socket connection | ||
| for addr in addr_info: | ||
| if addr[0] == socket.AF_INET6: | ||
| try: | ||
| test_socket = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) | ||
| test_socket.settimeout(2) | ||
| test_socket.close() | ||
| logger.info("✅ IPv6 connectivity appears to be available") | ||
| return True | ||
| except Exception as e: | ||
| logger.warning(f"⚠️ IPv6 address found but connectivity test failed: {e}") | ||
| return False | ||
| return True # If no IPv6, assume IPv4 works | ||
| except Exception as e: | ||
| logger.warning(f"⚠️ Could not check IPv6 connectivity: {e}") | ||
| return True # Assume it's okay and let the actual connection fail if needed | ||
|
|
There was a problem hiding this comment.
Fix the IPv6 connectivity test - currently doesn't test actual connectivity.
The function creates an IPv6 socket but immediately closes it without attempting to connect. This means it will always return True when IPv6 addresses are found, regardless of actual connectivity. The socket creation on line 42 merely validates that the OS supports IPv6, not that the host is reachable.
Apply this diff to actually test connectivity:
for addr in addr_info:
if addr[0] == socket.AF_INET6:
try:
test_socket = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
test_socket.settimeout(2)
+ # Actually test connectivity to the host
+ ipv6_addr = addr[4][0]
+ test_socket.connect((ipv6_addr, 5432)) # PostgreSQL default port
test_socket.close()
logger.info("✅ IPv6 connectivity appears to be available")
return True
except Exception as e:
logger.warning(f"⚠️ IPv6 address found but connectivity test failed: {e}")
+ test_socket.close()
return FalseAlternatively, consider whether this function should use the actual database port from db_config.port instead of hardcoding 5432.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.8)
46-46: Consider moving this statement to an else block
(TRY300)
47-47: Do not catch blind exception: Exception
(BLE001)
50-50: Consider moving this statement to an else block
(TRY300)
51-51: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In Backend/app/db/db.py around lines 30 to 54, the IPv6 check only creates and
immediately closes a socket so it doesn't verify reachability; replace that
logic to attempt an actual connect to the resolved IPv6 sockaddr and port:
iterate addr_info entries where addr[0] == socket.AF_INET6, extract the IPv6
sockaddr (addr[4]) and call sock.connect((ipv6_addr, port)) with a short
timeout, return True on first successful connect, log failures and try next
address, and return False only if all IPv6 addresses fail; ensure the socket is
closed in a finally block. Also use the real DB port (db_config.port) instead of
hardcoding 5432 and include informative logs on success/failure.
| if db_config.host and db_config.prefer_ipv4: | ||
| if not check_ipv6_connectivity(db_config.host): | ||
| logger.warning("⚠️ IPv6 connectivity issues detected. This is a known issue with some Supabase hosts.") | ||
| logger.info("💡 Consider using Supabase connection pooler or IPv4-compatible proxy") |
There was a problem hiding this comment.
Clarify the IPv6 check logic - the condition appears backwards.
The code checks IPv6 connectivity when prefer_ipv4 is True, which seems counterintuitive. If the user prefers IPv4, why check IPv6? Either:
- The condition should be inverted:
if db_config.host and not db_config.prefer_ipv4: - Or the intent is to warn users who set
prefer_ipv4=Truethat IPv6 issues were detected (defensive check)
Please clarify the intended behavior or fix the logic.
🤖 Prompt for AI Agents
In Backend/app/db/db.py around lines 93 to 96, the condition is reversed: we
should only check IPv6 connectivity when the user does NOT prefer IPv4. Change
the guard to run check_ipv6_connectivity only when db_config.host is set and
db_config.prefer_ipv4 is False (i.e., use "and not db_config.prefer_ipv4"); keep
the existing warning/info messages as-is.
| inspector = await conn.run_sync(lambda sync_conn: inspect(sync_conn)) | ||
| tables = await conn.run_sync(lambda sync_conn: inspect(sync_conn).get_table_names()) |
There was a problem hiding this comment.
Remove unused variable and duplicate inspect call.
inspector on line 67 is assigned but never used, and inspect() is called twice. Keep only the second call.
async with engine.connect() as conn:
# Check for required tables
- inspector = await conn.run_sync(lambda sync_conn: inspect(sync_conn))
tables = await conn.run_sync(lambda sync_conn: inspect(sync_conn).get_table_names())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| inspector = await conn.run_sync(lambda sync_conn: inspect(sync_conn)) | |
| tables = await conn.run_sync(lambda sync_conn: inspect(sync_conn).get_table_names()) | |
| async with engine.connect() as conn: | |
| # Check for required tables | |
| tables = await conn.run_sync(lambda sync_conn: inspect(sync_conn).get_table_names()) |
🧰 Tools
🪛 Ruff (0.14.8)
67-67: Local variable inspector is assigned to but never used
Remove assignment to unused variable inspector
(F841)
🤖 Prompt for AI Agents
In Backend/app/main.py around lines 67 to 68, remove the unused inspector
assignment and the duplicate inspect() call: delete the first line that assigns
inspector and keep only the second line that retrieves table names via
conn.run_sync(...). If you actually need an inspector object later, reuse the
single conn.run_sync(lambda sync_conn: inspect(sync_conn)) result instead of
calling inspect() twice.
1 participant
🎯 Overview
Implements a comprehensive database connectivity solution that eliminates server crashes caused by IPv6 DNS resolution issues, missing tables, and network configuration problems.
🐛 Problem Statement
Current Issues:
getaddrinfo failederrorstrending_nichestable causes API endpoint crashesRoot Causes:
✨ Solution Implemented
1. Enhanced Configuration Management (
config.py)2. Robust Database Connection (
db/db.py)3. Graceful Startup & Error Handling (
main.py)/and/healthfor monitoring4. Error-Resilient AI Routes (
routes/ai.py)5. Safe Database Seeding (
db/seed.py)6. Comprehensive Documentation
🔧 New Features
Configuration Options