JLL2-175: Better report when/if rows are skipped during subprocessing.

[adam-vessey]

Summary by CodeRabbit

• Bug Fixes
  • Enhanced migration skip handling to emit warning and notice messages when row skips are suppressed, replacing silent suppression with visible logging for improved debugging and visibility into migration execution.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

SubProcess plugin now catches MigrateSkipRowException during value processing and logs warning messages via the migration interface when propagateSkip is FALSE, instead of silently returning NULL. Processing continues without halting when skips are suppressed.

Changes

Cohort / File(s)	Summary
Skip Exception Handling & Messaging src/Plugin/migrate/process/SubProcess.php	Added try/catch blocks for MigrateSkipRowException in both transform() and doProcess() methods. When propagateSkip is FALSE, a warning/notice is logged via MigrationInterface and processing continues; when TRUE, the exception is rethrown. Imported MigrationInterface for message level constants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 A skip suppressed now speaks its truth,
With warnings logged in careful booth,
No silent failures in our sight,
Just gentle notes: "we'll be alright!"
Processing marches ever on... 🌾

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: improving reporting when rows are skipped during subprocessing, which aligns with the code changes that add warning/notice messages for skip-suppression handling.
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

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.}

[adam-vessey]

Hmm... aborting this for now. Things are made more complex when wanting to control what gets logged more granularly. For example, we probably do not want exceptions from a failed datastream lookup to be logged in all cases because it would be very spammy with optional datastreams; however, because we have it inside of a migration_lookup, and the migration_lookup plugin explicitly rethrows skip row exceptions explicitly instructing things not to be logged, we cannot cleanly control what gets logged based on the exception we raise, given we would typically be raising some form of skip row exception.

We could hypothetically get into a custom exception extending MigrateException, which migration_lookup would just rethrow, but such could get into spaghetti between how it gets raised in the DatastreamMapLookup plugin and how we would want to catch/log it indirectly in the subprocess plugin?