Repository navigation
Resend api added - #161
Resend api added#161
Conversation
WalkthroughSwitches email provider from SendGrid to Resend: adds a Resend-based sender and admin email endpoint, removes SendGrid sender and webhook controller, updates DI and project dependencies, adds Resend config keys, and modifies deploy workflow to inject the Resend secret. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as "Admin Client"
participant API as "Web.Api (AdminEmailController)"
participant Sender as "ResendEmailApiSender"
participant Resend as "Resend API"
rect rgba(0,128,0,0.5)
Admin->>API: POST /api/admin/emails/send (SendCustomEmailRequest)
end
rect rgba(0,0,255,0.5)
API->>Sender: Build EmailContent and call SendAsync
end
rect rgba(255,165,0,0.5)
Sender->>Resend: EmailSendAsync(EmailMessage) [To/Cc/Bcc, From, Subject, Html]
Resend-->>Sender: Send response (status/content)
end
Sender-->>API: Return result / log outcome
API-->>Admin: 200 OK (SendCustomEmailResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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
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: 3
🤖 Fix all issues with AI agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs`:
- Around line 70-75: The catch in ResendEmailApiSender (inside the
SendEmailAsync method) currently logs exceptions but swallows them; update the
method to surface failures by either rethrowing the exception after logging (use
"throw;" to preserve stack trace) or change the method signature and related
interface to return a result/boolean indicating success and return a failure
result after logging; ensure all callers are updated to handle the rethrown
exception or the new result type accordingly.
- Around line 46-62: In ResendEmailApiSender.cs the code calls email.Cc.Any()
and email.HiddenCc.Any() which can throw if those lists are null; update the
checks to guard for null (e.g., use email.Cc != null && email.Cc.Any() or
email.Cc?.Any() == true) and when iterating use a null-safe source (e.g.,
iterate over email.Cc ?? Enumerable.Empty<...>() or only enter the foreach after
the null check), keeping the existing assignments to message.Cc and message.Bcc;
reference symbols: email.Cc, email.HiddenCc, message.Cc, message.Bcc in the
sending method of the ResendEmailApiSender class.
- Around line 41-44: The loop that adds recipients to the MailMessage (the block
containing message.To.Add(recipient) in ResendEmailApiSender.cs) can throw a
NullReferenceException because EmailContent.Recipients may be null; add a
defensive null/empty check before iterating (e.g., guard with email.Recipients
!= null && email.Recipients.Any()) or iterate over email.Recipients ??
Enumerable.Empty<string>() so the foreach only runs when recipients exist;
ensure any needed System.Linq/System.Collections.Generic imports are present and
follow the same pattern used for Cc and HiddenCc.
🧹 Nitpick comments (1)
src/Infrastructure/Emails/ResendEmailApiSender.cs (1)
15-24: Consider injectingIResendinstead of creating it in the constructor.Creating the
ResendClientdirectly in the constructor violates dependency injection best practices and makes unit testing difficult. Consider registeringIResendin the DI container and injecting it.♻️ Suggested refactor
public ResendEmailApiSender( - IConfiguration configuration, + IResend client, ILogger<ResendEmailApiSender> logger) { _logger = logger; - var apiKey = configuration["ResendComApiKey"] - ?? throw new InvalidOperationException("ResendComApiKey configuration is missing"); - - _client = ResendClient.Create(apiKey); + _client = client; }Then register the client in
ServiceRegistration.cs:services.AddOptions(); services.AddHttpClient<ResendClient>(); services.Configure<ResendClientOptions>(o => { o.ApiToken = configuration["ResendComApiKey"] ?? throw new InvalidOperationException("ResendComApiKey configuration is missing"); }); services.AddTransient<IResend, ResendClient>(); services.AddTransient<IEmailApiSender, ResendEmailApiSender>();Based on coding guidelines requiring review of dependency injection best practices.
| foreach (var recipient in email.Recipients) | ||
| { | ||
| message.To.Add(recipient); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the ResendEmailApiSender.cs file
find . -name "ResendEmailApiSender.cs" -type fRepository: Techinterview-space/web-api
Length of output: 122
🏁 Script executed:
# Also search for the EmailContent class definition
fd "EmailContent" --type f -e csRepository: Techinterview-space/web-api
Length of output: 131
🏁 Script executed:
# Check for any file that might contain the email-related models
fd -e cs -x grep -l "Recipients" {} \;Repository: Techinterview-space/web-api
Length of output: 372
🏁 Script executed:
# Read the ResendEmailApiSender.cs file to see the context around lines 41-44
cat -n ./src/Infrastructure/Emails/ResendEmailApiSender.csRepository: Techinterview-space/web-api
Length of output: 2747
🏁 Script executed:
# Read the EmailContent.cs file to understand Recipients property
cat -n src/Infrastructure/Emails/Contracts/Requests/EmailContent.csRepository: Techinterview-space/web-api
Length of output: 1196
🏁 Script executed:
# Check if the project has nullable reference types enabled
cat -n src/Infrastructure/Emails/ResendEmailApiSender.cs | head -20Repository: Techinterview-space/web-api
Length of output: 805
🏁 Script executed:
# Check if there are any usage patterns or prior validation for Recipients
rg -n "Recipients" --type cs -B 2 -A 2 src/Web.Api/Services/Emails/TechInterviewerEmailService.csRepository: Techinterview-space/web-api
Length of output: 303
🏁 Script executed:
# Check the standard.ruleset to understand linting rules
find . -name "standard.ruleset" -o -name "*.ruleset" | head -5Repository: Techinterview-space/web-api
Length of output: 93
Add null/empty validation for Recipients to prevent NullReferenceException.
The Recipients collection can be null when EmailContent is instantiated via the parameterless constructor (e.g., ASP.NET model binding), and iterating over it without checking will throw. While Cc and HiddenCc are defensively validated with .Any() checks before iteration (lines 46, 55), Recipients lacks this protection.
Proposed fix
+ if (email.Recipients == null || !email.Recipients.Any())
+ {
+ throw new ArgumentException("Email must have at least one recipient", nameof(email));
+ }
+
foreach (var recipient in email.Recipients)
{
message.To.Add(recipient);
}📝 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.
| foreach (var recipient in email.Recipients) | |
| { | |
| message.To.Add(recipient); | |
| } | |
| if (email.Recipients == null || !email.Recipients.Any()) | |
| { | |
| throw new ArgumentException("Email must have at least one recipient", nameof(email)); | |
| } | |
| foreach (var recipient in email.Recipients) | |
| { | |
| message.To.Add(recipient); | |
| } |
🤖 Prompt for AI Agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs` around lines 41 - 44, The
loop that adds recipients to the MailMessage (the block containing
message.To.Add(recipient) in ResendEmailApiSender.cs) can throw a
NullReferenceException because EmailContent.Recipients may be null; add a
defensive null/empty check before iterating (e.g., guard with email.Recipients
!= null && email.Recipients.Any()) or iterate over email.Recipients ??
Enumerable.Empty<string>() so the foreach only runs when recipients exist;
ensure any needed System.Linq/System.Collections.Generic imports are present and
follow the same pattern used for Cc and HiddenCc.
| if (email.Cc.Any()) | ||
| { | ||
| message.Cc ??= new EmailAddressList(); | ||
| foreach (var cc in email.Cc) | ||
| { | ||
| message.Cc.Add(cc); | ||
| } | ||
| } | ||
|
|
||
| if (email.HiddenCc.Any()) | ||
| { | ||
| message.Bcc ??= new EmailAddressList(); | ||
| foreach (var bcc in email.HiddenCc) | ||
| { | ||
| message.Bcc.Add(bcc); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException when calling .Any() on null collections.
If email.Cc or email.HiddenCc are null, calling .Any() will throw. Consider using null-conditional or null-coalescing patterns.
🐛 Proposed fix
- if (email.Cc.Any())
+ if (email.Cc?.Any() == true)
{
message.Cc ??= new EmailAddressList();
foreach (var cc in email.Cc)
{
message.Cc.Add(cc);
}
}
- if (email.HiddenCc.Any())
+ if (email.HiddenCc?.Any() == true)
{
message.Bcc ??= new EmailAddressList();
foreach (var bcc in email.HiddenCc)
{
message.Bcc.Add(bcc);
}
}🤖 Prompt for AI Agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs` around lines 46 - 62, In
ResendEmailApiSender.cs the code calls email.Cc.Any() and email.HiddenCc.Any()
which can throw if those lists are null; update the checks to guard for null
(e.g., use email.Cc != null && email.Cc.Any() or email.Cc?.Any() == true) and
when iterating use a null-safe source (e.g., iterate over email.Cc ??
Enumerable.Empty<...>() or only enter the foreach after the null check), keeping
the existing assignments to message.Cc and message.Bcc; reference symbols:
email.Cc, email.HiddenCc, message.Cc, message.Bcc in the sending method of the
ResendEmailApiSender class.
| catch (Exception e) | ||
| { | ||
| _logger.LogError( | ||
| e, | ||
| "Could not send email via Resend"); | ||
| } |
There was a problem hiding this comment.
Swallowing exceptions silently hides email delivery failures.
The catch block logs the error but doesn't propagate it. Callers will have no way to know the email failed to send, which could lead to silent failures in critical notification flows (e.g., password resets, interview confirmations).
🐛 Proposed fix - rethrow or return a result
Option 1: Rethrow after logging:
catch (Exception e)
{
_logger.LogError(
e,
"Could not send email via Resend");
+ throw;
}Option 2: Return a result type that indicates success/failure (requires interface change).
📝 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.
| catch (Exception e) | |
| { | |
| _logger.LogError( | |
| e, | |
| "Could not send email via Resend"); | |
| } | |
| catch (Exception e) | |
| { | |
| _logger.LogError( | |
| e, | |
| "Could not send email via Resend"); | |
| throw; | |
| } |
🤖 Prompt for AI Agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs` around lines 70 - 75, The
catch in ResendEmailApiSender (inside the SendEmailAsync method) currently logs
exceptions but swallows them; update the method to surface failures by either
rethrowing the exception after logging (use "throw;" to preserve stack trace) or
change the method signature and related interface to return a result/boolean
indicating success and return a failure result after logging; ensure all callers
are updated to handle the rethrown exception or the new result type accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Web.Api/Features/Admin/AdminEmailController.cs`:
- Around line 25-49: SendCustomEmailAsync currently assumes
_emailApiSender.SendAsync always succeeds, but ResendEmailApiSender swallows
exceptions so the controller returns Success=true even on failure; update the
flow so send failures are propagated and surfaced: modify ResendEmailApiSender
to either throw the exception on failure or return a failure result, then update
SendCustomEmailAsync to await SendAsync and either catch exceptions and return
an error IActionResult (e.g., BadRequest or StatusCode(500) with Success=false
and a helpful Message) or inspect the returned result and return Success=false
when appropriate; reference SendCustomEmailAsync, _emailApiSender.SendAsync,
ResendEmailApiSender, and SendCustomEmailResponse when making the changes.
In `@src/Web.Api/Features/Salaries/UpdateSalary/UpdateSalaryHandler.cs`:
- Around line 37-43: currentUser may be null when calling
_auth.GetCurrentUserOrNullAsync, so update the logic in UpdateSalaryHandler to
handle that: either replace GetCurrentUserOrNullAsync with a non-null-returning
method like GetCurrentUserAsync (so the intent is explicit) or add an explicit
null check after calling GetCurrentUserOrNullAsync and throw an appropriate
authentication exception (e.g., UnauthorizedAccessException or a custom
NotAuthenticatedException) before evaluating currentUser.Has(Role.Admin) or
accessing currentUser.Id; ensure the check occurs immediately after the call to
avoid a NullReferenceException.
| [HttpPost("send")] | ||
| public async Task<IActionResult> SendCustomEmailAsync( | ||
| [FromBody] SendCustomEmailRequest request, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| var recipients = new List<string> { request.To }; | ||
|
|
||
| var cc = !string.IsNullOrWhiteSpace(request.Cc) | ||
| ? new List<string> { request.Cc } | ||
| : new List<string>(); | ||
|
|
||
| await _emailApiSender.SendAsync( | ||
| new EmailContent( | ||
| request.From, | ||
| request.Subject, | ||
| request.HtmlBody, | ||
| recipients, | ||
| cc), | ||
| cancellationToken); | ||
|
|
||
| return Ok(new SendCustomEmailResponse | ||
| { | ||
| Success = true, | ||
| Message = "Email sent successfully" | ||
| }); |
There was a problem hiding this comment.
Don’t return success when the send fails.
ResendEmailApiSender currently swallows exceptions and only logs them, so this action will always return Success=true even when the email was not sent. Please propagate failures (throw or return a result) and surface a non‑200 response when sending fails.
🔧 Proposed controller-side handling (pairs with making sender throw or return failure)
- await _emailApiSender.SendAsync(
- new EmailContent(
- request.From,
- request.Subject,
- request.HtmlBody,
- recipients,
- cc),
- cancellationToken);
-
- return Ok(new SendCustomEmailResponse
- {
- Success = true,
- Message = "Email sent successfully"
- });
+ try
+ {
+ await _emailApiSender.SendAsync(
+ new EmailContent(
+ request.From,
+ request.Subject,
+ request.HtmlBody,
+ recipients,
+ cc),
+ cancellationToken);
+
+ return Ok(new SendCustomEmailResponse
+ {
+ Success = true,
+ Message = "Email sent successfully"
+ });
+ }
+ catch (Exception)
+ {
+ return StatusCode(502, new SendCustomEmailResponse
+ {
+ Success = false,
+ Message = "Email send failed"
+ });
+ }📝 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.
| [HttpPost("send")] | |
| public async Task<IActionResult> SendCustomEmailAsync( | |
| [FromBody] SendCustomEmailRequest request, | |
| CancellationToken cancellationToken) | |
| { | |
| var recipients = new List<string> { request.To }; | |
| var cc = !string.IsNullOrWhiteSpace(request.Cc) | |
| ? new List<string> { request.Cc } | |
| : new List<string>(); | |
| await _emailApiSender.SendAsync( | |
| new EmailContent( | |
| request.From, | |
| request.Subject, | |
| request.HtmlBody, | |
| recipients, | |
| cc), | |
| cancellationToken); | |
| return Ok(new SendCustomEmailResponse | |
| { | |
| Success = true, | |
| Message = "Email sent successfully" | |
| }); | |
| [HttpPost("send")] | |
| public async Task<IActionResult> SendCustomEmailAsync( | |
| [FromBody] SendCustomEmailRequest request, | |
| CancellationToken cancellationToken) | |
| { | |
| var recipients = new List<string> { request.To }; | |
| var cc = !string.IsNullOrWhiteSpace(request.Cc) | |
| ? new List<string> { request.Cc } | |
| : new List<string>(); | |
| try | |
| { | |
| await _emailApiSender.SendAsync( | |
| new EmailContent( | |
| request.From, | |
| request.Subject, | |
| request.HtmlBody, | |
| recipients, | |
| cc), | |
| cancellationToken); | |
| return Ok(new SendCustomEmailResponse | |
| { | |
| Success = true, | |
| Message = "Email sent successfully" | |
| }); | |
| } | |
| catch (Exception) | |
| { | |
| return StatusCode(502, new SendCustomEmailResponse | |
| { | |
| Success = false, | |
| Message = "Email send failed" | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/Web.Api/Features/Admin/AdminEmailController.cs` around lines 25 - 49,
SendCustomEmailAsync currently assumes _emailApiSender.SendAsync always
succeeds, but ResendEmailApiSender swallows exceptions so the controller returns
Success=true even on failure; update the flow so send failures are propagated
and surfaced: modify ResendEmailApiSender to either throw the exception on
failure or return a failure result, then update SendCustomEmailAsync to await
SendAsync and either catch exceptions and return an error IActionResult (e.g.,
BadRequest or StatusCode(500) with Success=false and a helpful Message) or
inspect the returned result and return Success=false when appropriate; reference
SendCustomEmailAsync, _emailApiSender.SendAsync, ResendEmailApiSender, and
SendCustomEmailResponse when making the changes.
| var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken); | ||
|
|
||
| if (!currentUser.Has(Role.Admin) && | ||
| salary.UserId != currentUser.Id) | ||
| { | ||
| throw new ForbiddenException("You can only edit your own salary records"); | ||
| throw new NoPermissionsException("You can only edit your own salary records"); | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException when accessing currentUser.
The method GetCurrentUserOrNullAsync (by naming convention) can return null, but currentUser is used directly on lines 39-40 without a null check. If no authenticated user exists, this will throw a NullReferenceException.
🐛 Proposed fix: Add null check or throw appropriate exception
var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken);
+if (currentUser is null)
+{
+ throw new NoPermissionsException("Authentication required");
+}
if (!currentUser.Has(Role.Admin) &&
salary.UserId != currentUser.Id)
{
throw new NoPermissionsException("You can only edit your own salary records");
}Alternatively, if the endpoint is already protected by an [Authorize] attribute and currentUser is guaranteed to be non-null at this point, consider using a method like GetCurrentUserAsync that throws if the user is not authenticated, to make this expectation explicit and avoid the misleading OrNull suffix.
📝 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.
| var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken); | |
| if (!currentUser.Has(Role.Admin) && | |
| salary.UserId != currentUser.Id) | |
| { | |
| throw new ForbiddenException("You can only edit your own salary records"); | |
| throw new NoPermissionsException("You can only edit your own salary records"); | |
| } | |
| var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken); | |
| if (currentUser is null) | |
| { | |
| throw new NoPermissionsException("Authentication required"); | |
| } | |
| if (!currentUser.Has(Role.Admin) && | |
| salary.UserId != currentUser.Id) | |
| { | |
| throw new NoPermissionsException("You can only edit your own salary records"); | |
| } |
🤖 Prompt for AI Agents
In `@src/Web.Api/Features/Salaries/UpdateSalary/UpdateSalaryHandler.cs` around
lines 37 - 43, currentUser may be null when calling
_auth.GetCurrentUserOrNullAsync, so update the logic in UpdateSalaryHandler to
handle that: either replace GetCurrentUserOrNullAsync with a non-null-returning
method like GetCurrentUserAsync (so the intent is explicit) or add an explicit
null check after calling GetCurrentUserOrNullAsync and throw an appropriate
authentication exception (e.g., UnauthorizedAccessException or a custom
NotAuthenticatedException) before evaluating currentUser.Has(Role.Admin) or
accessing currentUser.Id; ensure the check occurs immediately after the call to
avoid a NullReferenceException.
Summary by CodeRabbit
New Features
Chores
Removed
✏️ Tip: You can customize this high-level summary in your review settings.