feat(backend): support smtp#11
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthroughバックエンドに非同期SMTPクライアントをLettreで実装し、設定と依存を追加、AppStateへ組み込んで起動時に初期化する変更でメール送信を可能にしました。 変更SMTP email送信機能統合
推定コード レビュー労力🎯 3 (Moderate) | ⏱️ ~20 minutes 詩
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/settings.rs (1)
4-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
SettingsのDebug派生で SMTP パスワードが漏えいし得ます。
Line 4のDebug派生により、Line 14のsmtp_passwordがログ出力経路で平文露出するリスクがあります。機密情報を含む設定型ではDebugを外すか、マスク実装にしてください。修正案(最小)
-#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Deserialize)] pub struct Settings {As per coding guidelines, 「認証・認可、テナント境界、SQL の正しさ、エラーハンドリング、非同期処理の安全性を優先して確認してください。」
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/settings.rs` around lines 4 - 15, The Settings struct derives Debug which can expose smtp_password in logs; remove Debug from #[derive(...)] on Settings and add a custom impl std::fmt::Debug for Settings that prints all fields but replaces smtp_password with a redacted value (e.g., "<redacted>" or masked length) so sensitive secrets are never emitted; update any tests or usages that relied on the automatic Debug impl to use the new masked Debug if needed and ensure the impl references Settings and smtp_password by name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/utils/smtp.rs`:
- Around line 25-27: The doctest examples in apps/backend/src/utils/smtp.rs
attempt real network/async operations and cause test failures; update the
Rustdoc code fences for the SmtpClient::new example and the async
client.send_email example to use a non-executing attribute (e.g., add "ignore"
or "no_run") so they are not run as doctests, and for the async example ensure
it is marked ignored because it references an undefined variable `client` and
uses `.await` outside an async context (do not change logic, just mark both
examples to not execute).
- Around line 36-39: The AsyncSmtpTransport send path lacks an explicit timeout;
wrap the call to mailer.send(...).await inside tokio::time::timeout with a
sensible Duration (make it configurable or use a constant like smtp_timeout) and
handle the Err(elapsed) case by returning or mapping a timeout error from your
send_email function; update any code that constructs the mailer
(AsyncSmtpTransport::<Tokio1Executor>::starttls_relay) to keep using the same
mailer but enforce timeout only around the mailer.send(...) await, and ensure
the function returns a clear timeout-oriented error instead of hanging
indefinitely.
---
Outside diff comments:
In `@apps/backend/src/settings.rs`:
- Around line 4-15: The Settings struct derives Debug which can expose
smtp_password in logs; remove Debug from #[derive(...)] on Settings and add a
custom impl std::fmt::Debug for Settings that prints all fields but replaces
smtp_password with a redacted value (e.g., "<redacted>" or masked length) so
sensitive secrets are never emitted; update any tests or usages that relied on
the automatic Debug impl to use the new masked Debug if needed and ensure the
impl references Settings and smtp_password by name.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 08eb9857-814f-4937-917e-86a971c31bf7
⛔ Files ignored due to path filters (1)
apps/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
apps/backend/.env.exampleapps/backend/Cargo.tomlapps/backend/src/lib.rsapps/backend/src/main.rsapps/backend/src/settings.rsapps/backend/src/utils/mod.rsapps/backend/src/utils/smtp.rs
There was a problem hiding this comment.
Pull request overview
This PR adds SMTP-based email sending support to the backend (aligning with the request for an email feature, e.g. Issue #6), wiring a new SmtpClient into application state and introducing SMTP configuration via environment-driven settings.
Changes:
- Added a new async SMTP utility (
SmtpClient) capable of sending plain-text or multipart (text+HTML) emails. - Extended
Settingsand.env.examplewith SMTP configuration fields and instantiated the SMTP client during startup. - Added new dependencies (notably
lettre) and updated the lockfile accordingly.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/backend/src/utils/smtp.rs | Introduces SmtpClient and email composition/sending logic (incl. timeout). |
| apps/backend/src/utils/mod.rs | Exposes the new smtp utility module. |
| apps/backend/src/settings.rs | Adds required SMTP-related settings fields loaded from environment. |
| apps/backend/src/main.rs | Creates SmtpClient at startup and injects it into AppState. |
| apps/backend/src/lib.rs | Extends AppState with an SMTP client handle. |
| apps/backend/Cargo.toml | Adds email-related crates (lettre, lettre_email). |
| apps/backend/Cargo.lock | Dependency graph update due to new crates. |
| apps/backend/.env.example | Documents SMTP env vars required by the new settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let _ = timeout(Duration::from_secs(30), self.mailer.send(email)) | ||
| .await | ||
| .map_err(|_| "SMTP send timeout")?; | ||
|
|
| use std::time::Duration; | ||
|
|
||
| /// SMTPクライアントを提供するモジュール | ||
|
|
| lettre = { version = "0.11", default-features = false, features = ["builder", "hostname", "smtp-transport","tokio1", "tokio1-rustls-tls"] } | ||
| lettre_email = "0.9" No newline at end of file |
| settings.smtp_port, | ||
| &settings.smtp_username, | ||
| &settings.smtp_password, | ||
| )?; |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/utils/smtp.rs`:
- Line 100: ハードコードされた30秒タイムアウトをモジュールレベルの定数に抽出してください:
timeout(Duration::from_secs(30), self.mailer.send(email)) の
Duration::from_secs(30) を例えば MAIL_SEND_TIMEOUT_SECS または MAIL_SEND_TIMEOUT
に置き換え、モジュール上部で定数として定義して参照するように修正してください(参照箇所: self.mailer.send(email) を囲む timeout
呼び出し)。
- Around line 100-102: The current call using `let _ =
timeout(Duration::from_secs(30), self.mailer.send(email)).await.map_err(|_|
"SMTP send timeout")?` discards the inner `Result<Response, SmtpError>` so real
SMTP errors are ignored; update the logic around `timeout`,
`self.mailer.send(email)` and the `await` to first handle the outer timeout
error, then inspect and propagate the inner `Result` from
`self.mailer.send(email)` (e.g., capture the outer result, then `map_err` or
`map` the inner `Result<Response, SmtpError>` into a proper error and `?` it) so
both timeout and SMTP send errors are returned instead of being dropped.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6554ab81-e1e8-4b6a-8e14-59e7f4291306
📒 Files selected for processing (1)
apps/backend/src/utils/smtp.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/settings.rs`:
- Around line 11-15: The SMTP fields (smtp_host, smtp_port, smtp_username,
smtp_password, smtp_from) are currently required and cause load_settings() to
fail in environments without SMTP; change these fields to optional (e.g.,
Option<String>/Option<u16>) or gate them behind a feature flag so settings
deserialization succeeds when SMTP is unused, update load_settings() to
accept/migrate optional values, and adjust any code that references these fields
(e.g., mail-sending helper functions) to handle None (skip sending or return a
clear error) and to validate presence only when SMTP is actually enabled.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 06ecc781-d283-4b17-b771-0e47a5b2c093
📒 Files selected for processing (1)
apps/backend/src/settings.rs
| pub smtp_host: String, | ||
| pub smtp_port: u16, | ||
| pub smtp_username: String, | ||
| pub smtp_password: String, | ||
| pub smtp_from: String, |
There was a problem hiding this comment.
SMTP設定を全必須にすると既存環境で起動不能になる可能性があります。
Line 11-15 の smtp_* がすべて必須のため、未設定環境では load_settings() のデシリアライズで即失敗します。段階的導入を想定するなら Option 化または feature flag で無効化可能にして、メール未使用時は起動継続できる形にしてください。
As per coding guidelines 「Rust / Axum / SeaORM のバックエンドです。認証・認可、テナント境界、SQL の正しさ、エラーハンドリング、非同期処理の安全性を優先して確認してください。」。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/settings.rs` around lines 11 - 15, The SMTP fields
(smtp_host, smtp_port, smtp_username, smtp_password, smtp_from) are currently
required and cause load_settings() to fail in environments without SMTP; change
these fields to optional (e.g., Option<String>/Option<u16>) or gate them behind
a feature flag so settings deserialization succeeds when SMTP is unused, update
load_settings() to accept/migrate optional values, and adjust any code that
references these fields (e.g., mail-sending helper functions) to handle None
(skip sending or return a clear error) and to validate presence only when SMTP
is actually enabled.
Docstrings generation was requested by @yupix. * #11 (comment) The following files were modified: * `apps/backend/src/main.rs` * `apps/backend/src/settings.rs` * `apps/backend/src/utils/smtp.rs`
Summary by CodeRabbit