"A(n) object was thrown while attempting to stub." Hard to debug migration message.

I ran into this issue. The current message really isn't helpful. The patch attached changes this:

service_page:localgov_services_parent:migration_lookup: A(n) object was thrown while attempting to stub.

To this:

service_page:localgov_services_parent:migration_lookup: A Drupal\Core\Database\DatabaseExceptionWrapper was thrown while attempting to stub, with message: SQLSTATE[22007]: Invalid datetime format: 1366 Incorrect integer value: '://client-d9.ddev.site/node/29' for column `db`.`migrate_map_service_landing_page`.`sourceid1` at row 1: INSERT INTO "migrate_map_service_landing_page" ("source_ids_hash", "sourceid1", "source_row_status", "rollback_action", "hash", "destid1") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5); Array
(
[:db_insert_placeholder_0] => 3f4361ef4d68dab9ed5847c92cb4c0471c04e54831787bb988664732d443eb0f
[:db_insert_placeholder_1] => ://client-d9.ddev.site/node/29
[:db_insert_placeholder_2] => 1
[:db_insert_placeholder_3] => 0
[:db_insert_placeholder_4] =>
[:db_insert_placeholder_5] => 25134
)

+++ b/core/modules/migrate/src/Plugin/migrate/process/MigrationLookup.php
@@ -263,7 +263,7 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+        throw new MigrateException(sprintf('A %s was thrown while attempting to stub, with message: %s', get_class($e), $e->getMessage()), $e->getCode(), $e);

This could be shortened to just '%s was thrown while attempting to stub: %s'

Is there a simple way of writing a test case that covers this?

Moving to NW for the comment #6. I don't know about a test case. Could we test the error message returned?

@longwave,

Made changes as per comment #6, but still, need work for the test case.

Updating issue metadata to reflect the need for tests.

I actually want to try to write these tests. Assigning to me.

Added a test case and updated issue summary

Oops sorry @danflanagan8 I didn't see that comment before starting to work on this.

Here's a test case to address this. I'm uploading a patch that runs just the test class that's modified in an attempt to be stingy with resources.

There is not much coverage for exception handling by the MigrationLookup process plugin. We could add a lot more coverage but this might not be the time or place to do it.

nm, I got scooped! I'm unassigning from myself and I guess I'll review instead. My test looked almost exactly like the one added in #11, but I did a couple things differently that I like.

First I used a new test method rather than adding to an existing one. These are unit tests, so we don't need to worry about saving processing time. I prefer the clarity that comes from a separate test. A separate test also allows for easy use of a dataProvider if we were to decide to test the other exception handling related to stubbing.

Second, I used an EntityStorageException instead of just an \Exception. I prefer EntityStorageException because it makes it clearer that the first word in the message is being populated based on the exception class. One could imagine that the word "Exception" is hardcoded at the beginning of the message.

I guess a third thing I did was to update drupalci.yml so that we save resources while running the test-only patch here. That's a really nice trick that I like share:

# This is the DrupalCI testbot build file for Drupal core.
# Learn to make one for your own drupal.org project:
# https://www.drupal.org/drupalorg/docs/drupal-ci/customizing-drupalci-testing
build:
  assessment:
    testing:
      # Run code quality checks.
      container_command.commit-checks:
        commands:
          - "core/scripts/dev/commit-code-check.sh --drupalci"
        halt-on-fail: true
      # run_tests task is executed several times in order of performance speeds.
      # halt-on-fail can be set on the run_tests tasks in order to fail fast.
      # suppress-deprecations is false in order to be alerted to usages of
      # deprecated code.
      run_tests.phpunit:
        types: 'PHPUnit-Unit'
        testgroups: '--class "Drupal\Tests\migrate\Unit\process\MigrationLookupTest"'
        suppress-deprecations: false
        halt-on-fail: false

I think at the end of the day, though, the test in #11 is good enough. I'll RTBC if things come back green!

Status got messed up during a cross post.

Setting back to NR because the tests are still running on #11.

Please ignore the patch in #13 which I did not mean to upload.

The patch in #11 looks great. Due to the cross posting (#13) though, I think I need to re-upload the patch from #11 here so that it's clear what patch I'm RTBC-ing. I downloaded the patch from #11, changed the name, and I am re-uploading here.

Also, for some reason (the cross post?) @smustgrave is not checked in the credits box. Let's make sure he gets his due! Thanks all!

Unrelated ckeditor5 fail. Resetting status to RTBC.

Unrelated Quick Edit fail this time, back to RTBC again.

Another ckeditor5 fail.

Committed and pushed 2e76fda4ff to 10.1.x and 599814852c to 10.0.x and 8671583c12 to 9.5.x and 6acd4fddc2 to 9.4.x. Thanks!

Backported this to 9.4.x as useful low risk bug fix.