Fix: Avoid raise condition con sending email inside atomic transaction#254
Merged
Conversation
mateodurante
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability of the asynchronous email-sending workflow by ensuring the Celery task is only dispatched after the EmailMessage row is safely committed, and it adds additional logging when the email record can’t be found after retries.
Changes:
- Defer
async_send_email.delay(...)until the surrounding DB transaction commits viatransaction.on_commit(...). - Add an error log when
async_send_emailexhausts retries without finding theEmailMessage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ngen/mailer/email_handler.py |
Defers dispatching the Celery send task until after DB commit to avoid race conditions. |
ngen/tasks.py |
Adds explicit error logging when an EmailMessage can’t be found after the maximum retries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mateodurante
added a commit
that referenced
this pull request
May 13, 2026
* feat: Implement Analyzer and AnalyzerMapping models with associated s… (#250) * feat: Implement Analyzer and AnalyzerMapping models with associated serializers and views - Added Analyzer model with fields for name, type, enabled status, config, and description. - Created AnalyzerMapping model with foreign key to Analyzer and fields for mapping_from and mapping_to. - Developed serializers for both models to handle validation and representation. - Introduced viewsets for Analyzer and AnalyzerMapping, including custom actions for testing connections and retrieving vulnerability choices. - Updated event retesting functionality to support analyzer mappings, allowing for dynamic selection of analyzers during event retests. - Enhanced frontend component to utilize dropdown for selecting analyzers when retesting events. - Added filters for Analyzer and AnalyzerMapping models to improve query capabilities. - Implemented migrations for new models and relationships, ensuring database integrity. - Created unit tests for Analyzer and AnalyzerMapping models to validate functionality and constraints. Co-authored-by: Copilot <copilot@github.com> * Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Update ngen/analyzers/kintun.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update frontend/src/views/analyzer/components/FormAnalyzer.jsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: Make analyzer selection required in Create and Edit Analyzer Mapping forms; update KintunAdapter to disable redirects and improve error logging Co-authored-by: Copilot <copilot@github.com> * feat: Refactor analyzer configuration to include port and SSL options; update translations and form handling --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * new evidence notification (#251) * cleanup translations (#252) * fix: agrega filtro select url (#253) * fix: agrega filtro select url * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix: Avoid raise condition con sending email inside atomic transaction (#254) * avoid raise condition con sending email inside atomic transaction * better traceback * tests fix for transaction on_commit * fix entity too large nginx upload file --------- Co-authored-by: Ulises Martín Cabrera <ucabrera@cert.unlp.edu.ar> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
This pull request makes improvements to the email sending workflow to ensure more reliable task execution and better error logging. The main changes focus on deferring the asynchronous email sending task until after the database transaction is committed, and enhancing error reporting for missing email messages.
Email sending workflow improvements:
email_handler.py, the call toasync_send_email.delayis now wrapped in atransaction.on_commitcallback to ensure the task is only triggered after the email message is successfully saved to the database. This prevents race conditions where the task could run before the email message exists.Error handling and logging:
tasks.py, improved error logging for missingEmailMessageobjects: if the message is not found after the maximum number of retries, an error is logged with details, aiding in debugging and monitoring.