DGI9-637: More specific logging for failed enqueues.

[adam-vessey]

This may be somewhat spammy, with things such as:

however, the additional verbosity should help identify underlying errors.

Summary by CodeRabbit

• Bug Fixes
  • Improved migration batch error handling: exceptions encountered during enqueueing are now recorded and later surfaced during batch preparation, preserving original error details for clearer failure messages.
  • Ensures successful migration path is unchanged while providing more informative feedback when migrations fail.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Capture exceptions during enqueue() in MigrateBatchExecutable, store them in a new protected property, and defer throwing until prepareBatch() where the stored exception is wrapped and rethrown (or a generic failure is thrown if none is stored).

Changes

Cohort / File(s)	Change Summary
Exception Capture & Propagation src/MigrateBatchExecutable.php	Added protected ?\Exception $enqueueException = NULL. Modified enqueue() to stash caught RequirementsException and source plugin exceptions into enqueueException. Modified prepareBatch() to check enqueueException and throw a wrapped "Migration failed" exception preserving the original when present; otherwise throw a generic migration failure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 I dug a little tunnel through the code tonight,
Stashed a pesky error out of sight.
Later I whispered it back with a polite bow,
"Migration failed" — but with its story now. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions 'More specific logging for failed enqueues' but the actual changes implement exception handling and propagation logic rather than logging improvements.	Update the title to accurately reflect the main change, such as 'DGI9-637: Capture and propagate enqueue exceptions in batch processing' or 'DGI9-637: Improve enqueue error handling and exception propagation'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

src/MigrateBatchExecutable.php (1)

185-190: Consider moving property declaration to the class property block.

The $enqueueException property is declared between methods, whereas other class properties ($queue, $idMapStatuses, $options) are grouped at the top of the class (lines 43-57). Moving this property there would improve consistency and readability.

Suggested location

Move the property declaration to lines 57-58, after the $options property:

  /**
   * The options passed.
   *
   * `@var` array
   */
  protected array $options;

  /**
   * Stash any exception encountered during enqueueing, so it can be reported.
   *
   * `@var` \Exception|null
   */
  protected ?\Exception $enqueueException = NULL;

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2ed23a0 and 243593a.

📒 Files selected for processing (1)

• src/MigrateBatchExecutable.php

🔇 Additional comments (3)

src/MigrateBatchExecutable.php (3)

162-166: LGTM!

The conditional exception handling properly wraps the original exception when available, providing the additional context mentioned in the PR objectives. The use of the previous: named argument correctly chains the exception for debugging.

---

230-232: LGTM!

Correctly stashes the RequirementsException for later propagation while preserving the existing error message display.

---

245-246: LGTM!

Correctly captures the source plugin exception for enhanced error reporting in prepareBatch().

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)

• JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/MigrateBatchExecutable.php (1)

185-190: Consider using \Throwable instead of \Exception for broader compatibility.

The enqueueException property is typed as ?\Exception, but using ?\Throwable would be more robust since it catches both Exception and Error types. This aligns with your existing formatException() method at line 277, which already accepts \Throwable.

🔎 Proposed refactor

  /**
   * Stash any exception encountered during enqueueing, so it can be reported.
   *
-  * @var \Exception|null
+  * @var \Throwable|null
   */
- protected ?\Exception $enqueueException = NULL;
+ protected ?\Throwable $enqueueException = NULL;

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6a6937a and 2ed23a0.

📒 Files selected for processing (1)

• src/MigrateBatchExecutable.php

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: PHPUnit / Drupal 10.5 | PHP 8.4
• GitHub Check: PHPUnit / Drupal 10.3 | PHP 8.2
• GitHub Check: PHPUnit / Drupal 10.4 | PHP 8.3
• GitHub Check: PHPUnit / Drupal 10.5 | PHP 8.3
• GitHub Check: PHPUnit / Drupal 10.4 | PHP 8.4

🔇 Additional comments (3)

src/MigrateBatchExecutable.php (3)

215-233: LGTM! Requirements exception properly captured.

The RequirementsException is correctly stored for later propagation in prepareBatch(), while still displaying the immediate error message to the user.

---

238-247: LGTM! Source plugin exception properly captured.

The exception from the source plugin is correctly stored for later propagation in prepareBatch(), while ensuring the migration status is reset and the error message is displayed.

---

162-166: The codebase already uses multiple PHP 8.0+ features (named parameters in NaiveFileCopy.php and match expressions in LockingMigrationLookup.php and FoxmlFile.php), confirming that PHP 8.0 is the minimum requirement. The named parameter syntax on line 164 is consistent with the existing codebase and does not require changes.

[github-actions]

Tag generated by PR: v3.18.3