Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors and exceptions not reported consistently during config-import #2709

Closed
danepowell opened this issue Apr 12, 2017 · 8 comments
Closed

Comments

@danepowell
Copy link
Contributor

danepowell commented Apr 12, 2017

On our site, we are using Lightning Media, which provides a few fields in its /config/install directory, such as field.field.media.document.field_media_in_library. Whenever this module is installed, the uuid for this field is going to be set to a random value.

We have previously installed this module and exported the configuration to our sites /config/default directory. Note that this exported configuration is identical to what's provided by Lightning Media, except our version of the field now has a uuid, as expected.

The problem comes when we install the site with the config-dir argument, i.e. drush si --config-dir=config/default. I would expect this to delete and then recreate the media_in_library field (because the uuid has changed from the default config). And indeed, the logs indicate that this is what appears to happen:

Synchronized configuration: delete field.field.media.document.field_media_in_library. [ok]
..
Synchronized configuration: create field.field.media.document.field_media_in_library. [ok]

However, despite what's indicated in the log, the media_in_library field doesn't exist on the site. It doesn't appear that it was ever actually recreated.

If we subsequently run a config-import, the field does finally get created successfully. But this shouldn't be necessary--I would expect Drush to handle it correctly as part of the initial config sync.

@weitzman
Copy link
Member

Actually processing the incoming configuration is the responsibility of Drupal core. The log messages you see are just Drush parrotting out what core is reporting. If the results seem wrong to you, you'll have to dig into the core sync service. Follow the money ...

@danepowell
Copy link
Contributor Author

I tried to eliminate Drush from the picture using these steps:

  1. Install the site without the config-dir argument
  2. Observe that the media_in_library field exists, as expected (but with a randomized UUID)
  3. Try to import ("synchronize") configuration from config/default via the Drupal UI.
  4. Observe that the import crashes with an exception like this:

A fatal error occurred: Exception thrown while performing a schema update. Cannot rename media__field_mime_type to field_deleted_data_14304c55cb: table field_deleted_data_14304c55cb already exists.

I suspect that if the import were ever to make it as far as importing field_media_in_library, it would crash with the same exception.

I suspect what's happening here is that Drupal has a major core bug that prevents it from deleting and recreating a field in the same process without an interim garbage collection step. I found and posted in this core issue that seems to be just such a bug.

What's odd is that when Drush imports the configuration, these exceptions don't get thrown, or they get swallowed somewhere along the way. I would consider that a bug, but it's actually a rather fortunate bug for the time being... otherwise there'd be no way to import configuration for most sites! @weitzman what's your opinion on this?

@weitzman
Copy link
Member

Thanks for digging in deeper.

It does sound like there are multiple bugs in play here. If you can isolate any to Drush, I'm eager to fix them. Drush doesn't deliberately swallow exceptions, FWIW.

@danepowell
Copy link
Contributor Author

danepowell commented Apr 17, 2017

We got bitten by another instance of this today. I still haven't identified the underlying bug in this case, but imports via the UI were failing with this error:

Unexpected error during import with operation rename for field-blah-blah: Attempt to create a field that doesn't exist on entity type blah blah

drush config-import did not report any errors, but also failed to actually perform the import. Again, it seems like errors and exceptions generated by Drupal during the import process are not bubbling up to Drush.

What's odd is that certain types of errors do definitely cause config-import to fail, as they should:

Drupal\Core\Config\ConfigImporterException: There were errors validating the config synchronization. in Drupal\Core\Config\ConfigImporter->validate() (line 728 of
/core/lib/Drupal/Core/Config/ConfigImporter.php).
The import failed due for the following reasons: Configuration blah depends on the blah configuration that will not exist after import.

@danepowell danepowell changed the title Site install with config-dir argument doesn't handle UUIDs well Errors and exceptions not reported consistently during config-import Apr 17, 2017
@weitzman
Copy link
Member

If you ever get a chance to dig in I'll point to

where we try to catch and rethrow Exceptions.

@danepowell
Copy link
Contributor Author

This should be fixed at least partially by #2713. Seems kind of obvious in hindsight, oh well :)

@weitzman thanks a lot for the guidance, it was just enough to get me on the right track.

I know this fixes at least some instances of unreported errors, I'm still trying to verify that there aren't any other places where exceptions are getting swallowed during the import process.

@weitzman
Copy link
Member

Thanks for digging in here. Lets keep talking in the PR.

@danepowell
Copy link
Contributor Author

I confirmed that this does fix all unreported errors/exceptions I've found so far, so I agree let's close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants