- 🇺🇸United States hongpong Philadelphia
adding more checks and exceptions for the other ones as well (attachments comments etc)
- Status changed to Needs work
over 1 year ago 7:07am 30 September 2023 - 🇺🇸United States hongpong Philadelphia
- 🇺🇸United States hongpong Philadelphia
user duplicates and migration group duplicates have been fixed in other issues so this issue is shrinking away...
- 🇳🇮Nicaragua dinarcon
The problem with duplicated migrations, due to using the same prefix, should also be solved with the fix from 🐛 Duplicative generated migration group causes uncaught error at review stage EntityStorageException Needs work
I tested this and I am not allow to submit the form is the prefix is already in used. We can probably improve the error message, but otherwise the name conflict is being detected.
- 🇺🇸United States hongpong Philadelphia
I think looking ahead we want to make sure these are caught not just in the UI wizard but also in the migration generator which could be later triggered from drush (after drush 12 is implemented!). In other words malformed elements could be coming in that UI measures cannot prevent. This was flagged by mikeryan on one of the oldest issues. #2742283: Validate configuration in WordPressMigrationGenerator::createMigrations → ..
to be fair this issue could be postponed until we have drush working again. ✨ Implement Drush 12+ on wordpress_migrate Active . I think adding exceptions, like in the WIP here, is still a good idea when entities are being stored and similar.
- 🇺🇸United States hongpong Philadelphia
Regarding catching exceptions which I think is generally a good idea and best practice. Just adding some links here to refer to later, re best practices and how other modules are doing it, what is around in the migrate subsystem and so on.
I had some old WIP patches on a couple issues in the project to add these - linked below. The API for exceptions and watchdog logging changed after 10.1.x so it is good we are switching to 10.2.x minimum.
- On branches 9.x to 11.x: https://api.drupal.org/api/drupal/namespace/Drupal%21migrate%21Plugin%21...
- badplugindefinitionexception: https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Plugi...
- https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Migra...
- https://api.drupal.org/api/drupal/namespace/Drupal%21migrate%21Exception... includes EntityValidationException and RequirementsException
- watchdog_exception was deprecated https://www.drupal.org/node/2932520 → . A number of classes that were using watchdog_exception() now explicitly inject a logger to be able to pass to \Drupal\Core\Utility\Error::logException()
- example implementation: https://git.drupalcode.org/project/webform/-/merge_requests/406/diffs
- https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Utility%2...
Code for adding log entries should now be like:
<?php use \Drupal\Core\Utility\Error; $logger = \Drupal::logger('modulename'); Error::logException($logger, $exception); ?>
- Exceptions also discussed in: 💬 XML (XHR) invalid characters issue Postponed: needs info within wordpress_migrate. another test I made WIP: 🐛 Duplicative generated migration group causes uncaught error at review stage EntityStorageException Needs work . using exceptions with the pluggable interface - some ideas: ✨ Support contrib wordpress data / custom data | Yoast SEO Needs review . duplicate user exception: #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry → . some notes here similar: 🐛 Better exception handling for if username does not exist Active
- Nice patch for showing lines of errors: https://www.drupal.org/files/issues/2020-07-30/2969551-25.patch → - via #2969551: Migrate messages from caught exceptions need file and line details →
- Way to do badplugindefinitionexception: #2908282: Throw exception for source plugins without a source_module property → see patch https://www.drupal.org/files/issues/2908282-83.patch →
- Main doc page about exceptions in coding standards: https://www.drupal.org/node/608166 →
- Old forum thread, adding this because it uses "throwable" as a catchall - https://www.drupal.org/forum/support/module-development-and-code-questio... →
- Example of how it was patched for 10.1+ this year in webform: 📌 [Drupal:10.1.x] watchdog_exception() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0 Fixed