Fix: Replace unsafe unwrap in Postgres connector to prevent panic

[Yuanjia-Lee]

Motivation

This PR addresses a stability issue identified in the Postgres data connector where database failures could cause the Router process to panic.

Previously, unwrap() was used directly on pool.get().await and client.execute(...). If the database connection failed or a query error occurred (e.g., constraint violation), the unhandled Err would crash the entire service.

Modifications

• Modified sgl-router/src/data_connector/postgres.rs:
  • Replaced .unwrap() with the ? operator for self.store.pool.get().await.
  • Replaced .unwrap() with the ? operator for client.execute(...).

This change ensures that database errors are propagated up the call stack as Result::Err instead of triggering a panic, allowing the router to handle failures gracefully.

Accuracy Tests

• This is a stability fix for error handling and does not affect model inference accuracy or kernel logic.

Benchmarking and Profiling

• No performance impact. This change only affects the error path when database operations fail.

Checklist

• Format your code according to the Format code with pre-commit.
• Add unit tests according to the Run and add unit tests. (N/A for this simple fix)
• Update documentation according to Write documentation. (N/A)
• Provide accuracy and speed benchmark results according to Test the accuracy and Benchmark the speed. (N/A)
• Follow the SGLang code style guidance.
• Work with maintainers to merge your PR. See the PR Merge Process

[gemini-code-assist]

Summary of Changes

Hello @Yuanjia-Lee, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness of the application by addressing a critical stability issue in the Postgres data connector. By transitioning from direct unwrap() calls to the ? operator for database operations, the system will now gracefully propagate errors, preventing service crashes due to unhandled database connection or query failures. This is a stability fix that does not impact performance or core logic.

Highlights

• Enhanced Error Handling: Replaced direct unwrap() calls with the ? operator in the Postgres data connector to ensure database errors are propagated gracefully instead of causing application panics.
• Improved Stability: This change prevents the Router process from crashing due to unhandled database connection failures or query errors (e.g., constraint violations).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly identifies a critical stability issue and takes the right step by replacing .unwrap() with error propagation using the ? operator. This is an excellent improvement to prevent the service from panicking due to database connection or query failures.

However, the changes as proposed will likely not compile. The ? operator requires an automatic conversion between error types (from deadpool_postgres::PoolError and tokio_postgres::Error to your ConversationStorageError), which does not appear to be defined. I've left specific comments with suggestions to fix this using .map_err(). For a cleaner, more idiomatic solution across the file, I recommend implementing the From trait on your storage error enums for these external error types.

More importantly, this file (postgres.rs) contains numerous other .unwrap() and .expect() calls that present the same panic risk. While this PR is a great start, I strongly recommend a follow-up effort to eliminate all remaining instances of unwrap() and expect() in this connector to make it fully robust.

[gemini-code-assist]

This change correctly identifies the need for error propagation, but it will likely cause a compilation error. The ? operator requires a conversion from deadpool_postgres::PoolError (the error type from pool.get()) to this function's error type, ConversationStorageError. Since there doesn't appear to be an impl From<deadpool_postgres::PoolError> for ConversationStorageError, the conversion fails.

A more idiomatic Rust solution would be to implement the From trait. However, for a localized fix, you can use map_err to perform the conversion explicitly.

Suggested change

	let client = store.pool.get().await?;
	let client = store.pool.get().await.map_err(|e| ConversationStorageError::StorageError(e.to_string()))?;

[gemini-code-assist]

Similar to the previous comment, this change will also likely fail to compile. The error from batch_execute is tokio_postgres::Error, which needs to be converted to ConversationStorageError for the ? operator to work.

You can fix this by explicitly mapping the error.

Suggested change

	.await?;
	.await.map_err(|e| ConversationStorageError::StorageError(e.to_string()))?;