Skip to content

Retry failed firestore transaction if re-run requested while check-run is not yet completed.#5003

Merged
ievdokdm merged 4 commits into
mainfrom
184215-retry-rerun
Mar 26, 2026
Merged

Retry failed firestore transaction if re-run requested while check-run is not yet completed.#5003
ievdokdm merged 4 commits into
mainfrom
184215-retry-rerun

Conversation

@ievdokdm
Copy link
Copy Markdown
Contributor

Retry failed firestore transaction if re-run requested while check-run is not yet completed.

Decreased delayFactor to default 200 milliseconds and increased maxAttempts from 8 by default to 10 to lover first retry delay and potentially decrease response while survive flood of change requested from scheduled to in progress.

fix: flutter/flutter#184215

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Hi @ievdokdm, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

@ievdokdm ievdokdm added autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD labels Mar 26, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

## 📋 Review Summary

This PR addresses an issue where Firestore transactions could fail during re-running failed jobs due to high contention. It correctly introduces a retry mechanism using package:retry and adjusts RetryOptions for more aggressive initial retries, which should improve reliability and performance under load.

🔍 General Feedback

  • The use of RetryOptions is consistent across the modified handlers.
  • Error handling in rerun_all_failed_jobs.dart was updated from a 200 OK response to an exception, which is more aligned with standard practices for indicating that an operation cannot be performed.
  • Consider simplifying the r.retry closures by removing redundant async/await keywords to make the code more idiomatic.
  • Minor inconsistency noted between NotFoundException and BadRequestException for similar error states across different handlers.

Comment thread app_dart/lib/src/request_handlers/rerun_all_failed_jobs.dart Outdated
Comment thread app_dart/lib/src/request_handlers/rerun_failed_job.dart Outdated
Comment thread app_dart/lib/src/request_handlers/rerun_all_failed_jobs.dart
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 26, 2026
@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit Bot commented Mar 26, 2026

autosubmit label was removed for flutter/cocoon/5003, because - The status or check suite test-app-dart has failed. Please fix the issues identified (or deflake) before re-applying this label.

@ievdokdm ievdokdm added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 26, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 26, 2026
@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit Bot commented Mar 26, 2026

autosubmit label was removed for flutter/cocoon/5003, because - The status or check suite test-app-dart has failed. Please fix the issues identified (or deflake) before re-applying this label.

@ievdokdm ievdokdm added autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD and removed CICD Run CI/CD labels Mar 26, 2026
@ievdokdm ievdokdm changed the title retry failed firestore transaction during re-run failed jobs Retry failed firestore transaction if re-run requested while check-run is not yet completed. Mar 26, 2026
Comment thread app_dart/test/request_handlers/rerun_all_failed_jobs_test.dart
@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit Bot commented Mar 26, 2026

autosubmit label was removed for flutter/cocoon/5003, because This PR has not met approval requirements for merging. Changes were requested by {eyebrowsoffire}, please make the needed changes and resubmit this PR.
The PR author is a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 26, 2026
@ievdokdm ievdokdm requested a review from eyebrowsoffire March 26, 2026 23:43
Copy link
Copy Markdown
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ievdokdm ievdokdm added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 26, 2026
@ievdokdm ievdokdm merged commit 66fbf98 into main Mar 26, 2026
49 checks passed
@ievdokdm ievdokdm deleted the 184215-retry-rerun branch March 31, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add retry on re-run

2 participants