fix(email): bypass Redis cache for Gmail token on init endpoint#2120
fix(email): bypass Redis cache for Gmail token on init endpoint#2120evanhutnik merged 3 commits intomainfrom
Conversation
WalkthroughThe PR adds a no-cache Gmail token flow: a new Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/cloud-storage/email_service/src/util/gmail/auth.rs`:
- Around line 108-148: Add tracing instrumentation to ensure consistent
observability: annotate both fetch_gmail_token_no_cache and
fetch_gmail_token_usercontext_response with #[tracing::instrument] (include
relevant fields like user IDs if desired) so their async execution and errors
are captured in traces; update the function signatures to keep attributes above
the async fn declarations and re-run tests to ensure no compile warnings from
unused parameters in the instrumented scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8d6f9ff-180f-4533-8352-e0facddf4514
📒 Files selected for processing (6)
rust/cloud-storage/email/src/inbound/axum/axum_impls.rsrust/cloud-storage/email/src/outbound/gmail_token_provider.rsrust/cloud-storage/email_service/src/api/email/init.rsrust/cloud-storage/email_service/src/api/email/mod.rsrust/cloud-storage/email_service/src/api/middleware/gmail_token.rsrust/cloud-storage/email_service/src/util/gmail/auth.rs
| /// Fetches a user's gmail token directly from the auth service, bypassing the Redis cache. | ||
| /// Used by the init endpoint where we always want a fresh token. | ||
| pub async fn fetch_gmail_token_no_cache( | ||
| user_context: &UserContext, | ||
| redis_client: &RedisClient, | ||
| auth_service_client: &AuthServiceClient, | ||
| ) -> Result<String, Response> { | ||
| let key = TokenCacheKey::new( | ||
| &user_context.fusion_user_id, | ||
| &user_context.user_id, | ||
| UserProvider::Gmail.as_str(), | ||
| ); | ||
|
|
||
| let conn = redis_client | ||
| .inner | ||
| .get_multiplexed_async_connection() | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error=?e, "unable to connect to redis"); | ||
| ( | ||
| StatusCode::INTERNAL_SERVER_ERROR, | ||
| Json(ErrorResponse { | ||
| message: "unable to get gmail access token", | ||
| }), | ||
| ) | ||
| .into_response() | ||
| })?; | ||
|
|
||
| email::outbound::fetch_gmail_access_token_no_cache(&key, &conn, auth_service_client) | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error=?e, "unable to get gmail access token from auth service"); | ||
| ( | ||
| StatusCode::INTERNAL_SERVER_ERROR, | ||
| Json(ErrorResponse { | ||
| message: "unable to get gmail access token", | ||
| }), | ||
| ) | ||
| .into_response() | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding #[tracing::instrument] for consistency.
The existing fetch_gmail_token_usercontext_response function (lines 82-106) doesn't have #[tracing::instrument], but similar functions like fetch_token_or_delete_on_revocation (line 21) and fetch_and_cache_google_public_keys (line 192) do. For observability consistency, consider adding instrumentation to both this new function and the existing fetch_gmail_token_usercontext_response.
♻️ Suggested instrumentation
+#[tracing::instrument(skip(redis_client, auth_service_client), err)]
pub async fn fetch_gmail_token_no_cache(
user_context: &UserContext,
redis_client: &RedisClient,
auth_service_client: &AuthServiceClient,
) -> Result<String, Response> {📝 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.
| /// Fetches a user's gmail token directly from the auth service, bypassing the Redis cache. | |
| /// Used by the init endpoint where we always want a fresh token. | |
| pub async fn fetch_gmail_token_no_cache( | |
| user_context: &UserContext, | |
| redis_client: &RedisClient, | |
| auth_service_client: &AuthServiceClient, | |
| ) -> Result<String, Response> { | |
| let key = TokenCacheKey::new( | |
| &user_context.fusion_user_id, | |
| &user_context.user_id, | |
| UserProvider::Gmail.as_str(), | |
| ); | |
| let conn = redis_client | |
| .inner | |
| .get_multiplexed_async_connection() | |
| .await | |
| .map_err(|e| { | |
| tracing::error!(error=?e, "unable to connect to redis"); | |
| ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| Json(ErrorResponse { | |
| message: "unable to get gmail access token", | |
| }), | |
| ) | |
| .into_response() | |
| })?; | |
| email::outbound::fetch_gmail_access_token_no_cache(&key, &conn, auth_service_client) | |
| .await | |
| .map_err(|e| { | |
| tracing::error!(error=?e, "unable to get gmail access token from auth service"); | |
| ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| Json(ErrorResponse { | |
| message: "unable to get gmail access token", | |
| }), | |
| ) | |
| .into_response() | |
| }) | |
| } | |
| /// Fetches a user's gmail token directly from the auth service, bypassing the Redis cache. | |
| /// Used by the init endpoint where we always want a fresh token. | |
| #[tracing::instrument(skip(redis_client, auth_service_client), err)] | |
| pub async fn fetch_gmail_token_no_cache( | |
| user_context: &UserContext, | |
| redis_client: &RedisClient, | |
| auth_service_client: &AuthServiceClient, | |
| ) -> Result<String, Response> { | |
| let key = TokenCacheKey::new( | |
| &user_context.fusion_user_id, | |
| &user_context.user_id, | |
| UserProvider::Gmail.as_str(), | |
| ); | |
| let conn = redis_client | |
| .inner | |
| .get_multiplexed_async_connection() | |
| .await | |
| .map_err(|e| { | |
| tracing::error!(error=?e, "unable to connect to redis"); | |
| ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| Json(ErrorResponse { | |
| message: "unable to get gmail access token", | |
| }), | |
| ) | |
| .into_response() | |
| })?; | |
| email::outbound::fetch_gmail_access_token_no_cache(&key, &conn, auth_service_client) | |
| .await | |
| .map_err(|e| { | |
| tracing::error!(error=?e, "unable to get gmail access token from auth service"); | |
| ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| Json(ErrorResponse { | |
| message: "unable to get gmail access token", | |
| }), | |
| ) | |
| .into_response() | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/email_service/src/util/gmail/auth.rs` around lines 108 -
148, Add tracing instrumentation to ensure consistent observability: annotate
both fetch_gmail_token_no_cache and fetch_gmail_token_usercontext_response with
#[tracing::instrument] (include relevant fields like user IDs if desired) so
their async execution and errors are captured in traces; update the function
signatures to keep attributes above the async fn declarations and re-run tests
to ensure no compile warnings from unused parameters in the instrumented scope.
|
|
||
| /// Fetches a user's gmail token directly from the auth service, bypassing the Redis cache. | ||
| /// Used by the init endpoint where we always want a fresh token. | ||
| pub async fn fetch_gmail_token_no_cache( |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/cloud-storage/email_service/src/api/middleware/gmail_token.rs`:
- Around line 29-35: The tracing instrumentation on the Result-returning
function attach_gmail_token_no_cache should include error capture; update the
attribute on the function declaration (#[tracing::instrument(...)] applied to
attach_gmail_token_no_cache) to add err alongside the existing skip list (e.g.,
#[tracing::instrument(skip(ctx, user_context, req, next), err)]) so tracing
records error outcomes from the function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb109526-59fe-41e0-b39e-8d4b85cc3941
📒 Files selected for processing (3)
rust/cloud-storage/email/src/outbound/gmail_token_provider.rsrust/cloud-storage/email_service/src/api/middleware/gmail_token.rsrust/cloud-storage/email_service/src/util/gmail/auth.rs
| #[tracing::instrument(skip(ctx, user_context, req, next))] | ||
| pub(in crate::api) async fn attach_gmail_token_no_cache( | ||
| State(ctx): State<ApiContext>, | ||
| user_context: Extension<UserContext>, | ||
| mut req: Request, | ||
| next: Next, | ||
| ) -> Result<Response, Response> { |
There was a problem hiding this comment.
Add err to #[tracing::instrument] for Result-returning function.
The function returns Result<Response, Response>, so per coding guidelines, the #[tracing::instrument] attribute should include err to capture error outcomes in traces.
🔧 Proposed fix
-#[tracing::instrument(skip(ctx, user_context, req, next))]
+#[tracing::instrument(skip(ctx, user_context, req, next), err)]
pub(in crate::api) async fn attach_gmail_token_no_cache(As per coding guidelines: "Include err when adding the tracing::instrument attribute to functions that return Result."
📝 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.
| #[tracing::instrument(skip(ctx, user_context, req, next))] | |
| pub(in crate::api) async fn attach_gmail_token_no_cache( | |
| State(ctx): State<ApiContext>, | |
| user_context: Extension<UserContext>, | |
| mut req: Request, | |
| next: Next, | |
| ) -> Result<Response, Response> { | |
| #[tracing::instrument(skip(ctx, user_context, req, next), err)] | |
| pub(in crate::api) async fn attach_gmail_token_no_cache( | |
| State(ctx): State<ApiContext>, | |
| user_context: Extension<UserContext>, | |
| mut req: Request, | |
| next: Next, | |
| ) -> Result<Response, Response> { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/email_service/src/api/middleware/gmail_token.rs` around
lines 29 - 35, The tracing instrumentation on the Result-returning function
attach_gmail_token_no_cache should include error capture; update the attribute
on the function declaration (#[tracing::instrument(...)] applied to
attach_gmail_token_no_cache) to add err alongside the existing skip list (e.g.,
#[tracing::instrument(skip(ctx, user_context, req, next), err)]) so tracing
records error outcomes from the function.
2 participants
Summary
fetch_gmail_access_token_no_cachepath that always fetches a fresh Gmail token from the auth service, skipping the Redis cache read but still writing the token back to the cache after fetching.attach_gmail_token_no_cachemiddleware used only by the/email/initendpoint. All other endpoints continue using the existing cached middleware.gmail_token_provider.rsto extract shared helpers (fetch_token_from_auth_service,cache_token_in_redis) used by both the cached and no-cache flows.fetch_gmail_access_token_no_cacheto theGmailTokenProvidertrait and its implementation.