Conversation
Contributor
WalkthroughAdds a stateful EmailService (Resend client + templates) and integrates it into AppState; registration now sends verification emails and, on send failure, logs the error and compensatingly deletes the created user; adds email template utilities, test wiring, docs updates, and a urlencoding dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as Auth Handler
participant DB as Database
participant EmailSvc as EmailService
participant Resend as Resend API
Client->>Handler: POST /register (credentials)
Handler->>DB: create_user(...) -> returns user_id + token
Handler->>EmailSvc: send_verification_email(email, token)
EmailSvc->>EmailSvc: build_verification_html/text(verify_url)
EmailSvc->>Resend: send(CreateEmailOptions)
alt Email Send Success
Resend-->>EmailSvc: Ok
EmailSvc-->>Handler: Ok
Handler-->>Client: 201 Created
else Email Send Failure
Resend-->>EmailSvc: Err
EmailSvc-->>Handler: Err(AppError)
Handler->>DB: delete_user(user_id)
DB-->>Handler: Ok
Handler-->>Client: 500 Internal Server Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust-server/documentation.md (1)
299-331:⚠️ Potential issue | 🟡 MinorOutdated: email.rs documentation still describes placeholder implementation.
Lines 312-313 say "Currently a placeholder implementation (logs to stderr)" and the code sample shows
eprintln!. But you've now implemented real email sending via Resend! This section needs updating to reflect the actualEmailServiceimplementation.📚 Suggested documentation update
The
<details>block foremail.rs(lines 299-331) should be updated to describe:
- The
EmailServicestruct wrapping the Resend client- The
send_verification_email()method- Configuration requirements (API key, frontend URL, domain)
- The multipart email (HTML + plain text)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-server/documentation.md` around lines 299 - 331, The documentation still describes a placeholder eprintln! implementation for email.rs but the code now uses the EmailService struct wrapping the Resend client and a real send_verification_email method; update the details block to describe EmailService (wrapping Resend), the send_verification_email(&self, to_email: &str, token: &str) async method, required configuration (Resend API key, FRONTEND_URL for the verification link, and sending domain), and that the message is sent as multipart (HTML + plain text) rather than logging to stderr so readers know how to configure and use it.
🤖 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-server/src/features/auth/handlers.rs`:
- Around line 54-65: The compensating delete_user call can mask the original
email error because the `?` on delete_user will propagate a DB error instead of
returning the original `email_err`; update the block around
`state.email.send_verification_email(...)` so that on email send failure you (1)
log the email error with a clearer message (e.g., "Failed to send verification
email: {:#?}"), (2) attempt the compensating `delete_user(db, user_id).await`,
but do not use `?` — instead capture its Result and if it Errs, log that
compensation failure (e.g., "Failed to delete user during compensation: {:#?}"),
and (3) always return the original `email_err` from the handler; keep the
`is_test` check and references to `send_verification_email` and `delete_user`
intact.
In `@rust-server/src/utils/email_templates.rs`:
- Around line 62-70: The plain-text verification template in
build_verification_text references a non-existent "button"; update the wording
so it only mentions the link (e.g., "If the link above does not work, copy and
paste it into your browser" or "If you cannot click the link, copy the URL into
your browser") by editing the formatted string returned by
build_verification_text to replace the "button" sentence with link-appropriate
phrasing while keeping the rest of the text unchanged.
- Around line 1-60: The template in build_verification_html contains
inconsistent branding: "Tärpit" in the header and "Tarpit" in the footer; update
the text inside the HTML string so the brand name is the same in both places
(choose the intended spelling and replace occurrences of "Tärpit" and "Tarpit"
inside the format! string literal passed to build_verification_html to ensure
consistency).
In `@rust-server/src/utils/email.rs`:
- Line 50: The tracing::info! call currently logs the raw PII variable to_email;
change the log to avoid raw email addresses by deriving a non-identifying value
(e.g., mask the local part, extract only the domain, or use a user_id) before
logging. Locate the tracing::info!(to = to_email, "Verification email sent")
call in email.rs and replace the to= to_email field with a safe value produced
by a small helper (e.g., mask_email(to_email) or extract_domain(to_email) or use
an available user_id), ensuring the helper is implemented near send/verify
functions and used consistently for success/failure logs. Ensure the log message
still provides context (e.g., "Verification email sent" with masked_email or
domain) without emitting the full email.
- Around line 28-32: The verification link currently interpolates token raw into
verify_url (see verify_url, self.frontend_url, token); URL-encode the token
before interpolation to avoid broken links when it contains reserved characters.
Update the code that builds verify_url to compute an encoded_token (e.g., via
percent-encoding or urlencoding crate) and use that encoded_token in the format
call so the final query string is safe and correctly parsed.
In `@rust-server/tests/api/common.rs`:
- Around line 58-70: The EmailService::new call is duplicated in
build_test_server and build_test_server_without_db; extract it into a small
helper like create_test_email_service(config: &Config) that returns EmailService
and replace both constructors with email: create_test_email_service(&config) to
centralize initialization and ease future changes or mocking.
---
Outside diff comments:
In `@rust-server/documentation.md`:
- Around line 299-331: The documentation still describes a placeholder eprintln!
implementation for email.rs but the code now uses the EmailService struct
wrapping the Resend client and a real send_verification_email method; update the
details block to describe EmailService (wrapping Resend), the
send_verification_email(&self, to_email: &str, token: &str) async method,
required configuration (Resend API key, FRONTEND_URL for the verification link,
and sending domain), and that the message is sent as multipart (HTML + plain
text) rather than logging to stderr so readers know how to configure and use it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8341c574-47c0-4311-9035-bf167a0ff5c6
📒 Files selected for processing (9)
rust-server/documentation.mdrust-server/src/features/auth/handlers.rsrust-server/src/features/auth/queries.rsrust-server/src/main.rsrust-server/src/state.rsrust-server/src/utils/email.rsrust-server/src/utils/email_templates.rsrust-server/src/utils/mod.rsrust-server/tests/api/common.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
rust-server/src/features/auth/handlers.rs (1)
54-64:⚠️ Potential issue | 🟠 MajorPreserve the original email failure when compensation cleanup fails.
Line 63 still uses
?, so adelete_userDB error can maskemail_err. Log cleanup failure, but always return the original email error.Suggested fix
if !state.config.app_env.is_test() && let Err(email_err) = state .email .send_verification_email(&cleaned_data.email, &token) .await { // Compensating action: delete the user we just created tracing::error!("Failed to send verification email: {:#?}", email_err); - delete_user(db, user_id).await?; + if let Err(delete_err) = delete_user(db, user_id).await { + tracing::error!( + user_id = %user_id, + ?delete_err, + "Failed to delete user during compensation" + ); + } return Err(email_err); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-server/src/features/auth/handlers.rs` around lines 54 - 64, The current compensating cleanup in handlers.rs calls delete_user(db, user_id).await? which can propagate a DB error and mask the original email send failure stored in email_err; change that so delete_user is awaited and any error is logged (e.g., with tracing::error! including the delete error and context) but not returned — always return the original email_err from the send_verification_email failure in the block that handles Err(email_err) from send_verification_email; reference send_verification_email, email_err, and delete_user to locate the change.rust-server/src/utils/email.rs (1)
51-51:⚠️ Potential issue | 🟠 MajorAvoid logging raw recipient email addresses.
Line 51 logs PII (
to_email) directly. Log a masked value or domain instead.Suggested fix
- tracing::info!(to = to_email, "Verification email sent"); + let recipient_domain = to_email.split('@').nth(1).unwrap_or("unknown"); + tracing::info!(recipient_domain, "Verification email sent");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-server/src/utils/email.rs` at line 51, The tracing::info! call logs the raw recipient email (to_email); change it to log a non-PII masked value instead (e.g., mask the local-part or log only the domain). Implement or call a helper like mask_email(to_email) or extract_domain(to_email) and use that masked/domain value in the tracing::info! invocation (the call referencing to_email in utils/email.rs, e.g., in the send_verification_email function), so the log no longer contains the raw email address.rust-server/src/utils/email_templates.rs (1)
18-49:⚠️ Potential issue | 🟡 MinorUnify brand spelling in the HTML email copy.
Line 18 and Line 49 use different spellings (“Tärpit” vs “Tarpit”). Pick one brand form and keep it consistent.
Suggested fix
- <p style="margin:0;color:`#4b5563`;font-size:13px;line-height:1.6;">Thank you for joining Tarpit. See you inside.</p> + <p style="margin:0;color:`#4b5563`;font-size:13px;line-height:1.6;">Thank you for joining Tärpit. See you inside.</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-server/src/utils/email_templates.rs` around lines 18 - 49, The email HTML uses two different brand spellings ("Tärpit" in the <h1> Welcome heading and "Tarpit" in the closing paragraph); pick one canonical spelling and make them consistent in the template(s) in rust-server/src/utils/email_templates.rs by updating the closing copy ("Thank you for joining Tarpit. See you inside.") to match the chosen brand form (or update the <h1> if you prefer the plain ASCII form), ensuring all occurrences in this file use the same string.
🤖 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-server/documentation.md`:
- Around line 72-78: The documentation table is missing the email_templates.rs
entry; update the table row list in documentation.md to include
`email_templates.rs` with a short purpose description like "Email templating
utilities for transactional and notification messages" so it appears alongside
`email.rs`, `password.rs`, `validators.rs`, etc.; ensure the new row uses the
same pipe-delimited Markdown format as the other entries and is placed within
the same table block shown in the diff.
---
Duplicate comments:
In `@rust-server/src/features/auth/handlers.rs`:
- Around line 54-64: The current compensating cleanup in handlers.rs calls
delete_user(db, user_id).await? which can propagate a DB error and mask the
original email send failure stored in email_err; change that so delete_user is
awaited and any error is logged (e.g., with tracing::error! including the delete
error and context) but not returned — always return the original email_err from
the send_verification_email failure in the block that handles Err(email_err)
from send_verification_email; reference send_verification_email, email_err, and
delete_user to locate the change.
In `@rust-server/src/utils/email_templates.rs`:
- Around line 18-49: The email HTML uses two different brand spellings ("Tärpit"
in the <h1> Welcome heading and "Tarpit" in the closing paragraph); pick one
canonical spelling and make them consistent in the template(s) in
rust-server/src/utils/email_templates.rs by updating the closing copy ("Thank
you for joining Tarpit. See you inside.") to match the chosen brand form (or
update the <h1> if you prefer the plain ASCII form), ensuring all occurrences in
this file use the same string.
In `@rust-server/src/utils/email.rs`:
- Line 51: The tracing::info! call logs the raw recipient email (to_email);
change it to log a non-PII masked value instead (e.g., mask the local-part or
log only the domain). Implement or call a helper like mask_email(to_email) or
extract_domain(to_email) and use that masked/domain value in the tracing::info!
invocation (the call referencing to_email in utils/email.rs, e.g., in the
send_verification_email function), so the log no longer contains the raw email
address.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4bb5769-0fe8-4f67-8ffc-acfb879e0074
⛔ Files ignored due to path filters (1)
rust-server/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
rust-server/Cargo.tomlrust-server/documentation.mdrust-server/src/features/auth/handlers.rsrust-server/src/utils/email.rsrust-server/src/utils/email_templates.rs
Contributor
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-server/src/features/auth/handlers.rs`:
- Around line 63-69: delete_user currently can return Ok(()) even when no DB row
was deleted, so the compensation path in handlers.rs may silently fail; change
delete_user (in rust-server/src/features/auth/queries.rs) to check the SQL
result's rows_affected() == 1 and return an Err variant (e.g., a NotFound or
CompensationFailed error in your auth error enum) when zero rows were deleted,
and update the call sites (e.g., the compensation block in handlers.rs that
calls delete_user(db, user_id).await) to handle that error accordingly so
missing-deletion becomes observable/actionable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e4ecc291-8f3c-44f2-9c09-0ce5e88591ab
📒 Files selected for processing (1)
rust-server/src/features/auth/handlers.rs
Comment on lines
+63
to
+69
| if let Err(delete_err) = delete_user(db, user_id).await { | ||
| tracing::error!( | ||
| user_id = %user_id, | ||
| ?delete_err, | ||
| "Failed to delete user during compensation" | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Compensation success is still not fully verified.
At Line 63, delete_user(db, user_id) can return Ok(()) even when no row is deleted (see rust-server/src/features/auth/queries.rs Lines 52-63). That can silently leave a created account behind after email failure.
Please make delete_user validate rows_affected() == 1 (or return a typed not-found/compensation error) so this path is observable and actionable.
Suggested follow-up change (in rust-server/src/features/auth/queries.rs)
pub async fn delete_user<'e, E>(db: E, user_id: Uuid) -> Result<(), AppError>
where
E: Executor<'e, Database = Postgres>,
{
- sqlx::query("DELETE FROM users WHERE id = $1")
+ let result = sqlx::query("DELETE FROM users WHERE id = $1")
.bind(user_id)
.execute(db)
.await
.map_err(AppError::Sql)?;
+ if result.rows_affected() != 1 {
+ return Err(AppError::internal("Compensation delete did not remove expected user row"));
+ }
+
Ok(())
}📝 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.
Suggested change
| if let Err(delete_err) = delete_user(db, user_id).await { | |
| tracing::error!( | |
| user_id = %user_id, | |
| ?delete_err, | |
| "Failed to delete user during compensation" | |
| ); | |
| } | |
| pub async fn delete_user<'e, E>(db: E, user_id: Uuid) -> Result<(), AppError> | |
| where | |
| E: Executor<'e, Database = Postgres>, | |
| { | |
| let result = sqlx::query("DELETE FROM users WHERE id = $1") | |
| .bind(user_id) | |
| .execute(db) | |
| .await | |
| .map_err(AppError::Sql)?; | |
| if result.rows_affected() != 1 { | |
| return Err(AppError::internal("Compensation delete did not remove expected user row")); | |
| } | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-server/src/features/auth/handlers.rs` around lines 63 - 69, delete_user
currently can return Ok(()) even when no DB row was deleted, so the compensation
path in handlers.rs may silently fail; change delete_user (in
rust-server/src/features/auth/queries.rs) to check the SQL result's
rows_affected() == 1 and return an Err variant (e.g., a NotFound or
CompensationFailed error in your auth error enum) when zero rows were deleted,
and update the call sites (e.g., the compensation block in handlers.rs that
calls delete_user(db, user_id).await) to handle that error accordingly so
missing-deletion becomes observable/actionable.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests