Problem/motivation

If a media type's source field is deleted, fatal errors can occur on the status report page.

This is mostly by design; the media system assumes the source field exists and does not try to gracefully handle missing source fields. That said, a missing source field probably shouldn't cause a fatal error on the status report. It's one thing for that to happen on pages that are specific to the media system, but our code could be a little more graceful in hook_requirements().

Proposed resolution

In media_requirements(), silently skip over media types whose source field is missing. In #3205866: media_requirements() should report missing source fields, we should actually make the hook report the missing source fields as actionable errors.

Remaining tasks

Open the aforementioned follow-up, then review and commit the patch.

API changes

None.

Data model changes

None.

UI changes

None.

Release notes snippet

None needed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Drupaldev2013 created an issue. See original summary.

Drupaldev2013’s picture

FileSize
759 bytes

I have applied this patch and It's work

joekers’s picture

I had this issue when some of the config for a new media type hadn't been deployed properly so should try looking at that first.

I don't think the patch is needed but if it were you should take a look at the coding standards:
https://www.drupal.org/docs/develop/standards/coding-standards#controlst...

cilefen’s picture

Title: Media in drupal core » Error: Call to a member function getItemDefinition() on null in media_requirements()
Version: 8.1.x-dev » 8.8.x-dev
Issue tags: -Error: Call to a member function getItemDefinition() on null in media_requirements()
rajanvalecha12’s picture

The patch in #2 didn't apply correctly to either core release 8.8.1 or 8.8.x.
Thus adding a patch as per 8.8.x.

Status: Needs review » Needs work

The last submitted patch, 5: 3106659-5.patch, failed testing. View results

aleevas’s picture

Status: Needs work » Needs review
FileSize
3.64 KB
863 bytes

I tried to fix last patch

audacus’s picture

I had to add another check because the source field definitions were null for the type media_entity_slideshare of the module Media entity slideshare.

Status: Needs review » Needs work

The last submitted patch, 8: 3106659-8.patch, failed testing. View results

aleevas’s picture

Version: 8.8.x-dev » 8.9.x-dev
Status: Needs work » Needs review

Seems to it's works now for both version: 8.8 and 8.9

ddhuri’s picture

#8 works for me.

Thanks.

mulukallaarun’s picture

After upgrade from Drupal 8.7.11 to 8.8.4, I also faced this error:
Error: call to a member function getitemdefinition() on null in media_requirements() (line 140 of /core/modules/media/media.install).
I had used the attached patch if it can help others.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

cobenash’s picture

#12 works for me.

Nicolas S.’s picture

Patch #12 works too for me

phenaproxima credited dpi.

phenaproxima’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Bug Smash Initiative, +Needs tests
Related issues: +#3145098: Media requirements exception when field for a media type does not exist.

Transferring credit from #3145098: Media requirements exception when field for a media type does not exist., which has been closed as a duplicate of this one. I am also copying that issue's summary into here, and tagging this issue for test coverage.

phenaproxima’s picture

IMHO this is a major bug that breaks things for site builders and developers, so bumping priority.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

RobbMLewis’s picture

Patch 12 works for me. This does break the site unless applied. Specifically, it kills Commerce. for my site. Can this get bumped up and put into core?

kndr’s picture

@phenaproxima said in the comment https://www.drupal.org/project/drupal/issues/3145098#comment-13773168

So: the thing is, any media type's source field is needed for that media type to work at all. It is a foundational assumption of the media API that, if the media type has been saved, the source field exists. If it doesn't, that's a major error condition and many more things could be expected to break. Badly.

If it is a foundational assumption of the media API (I totally agree with that) so how can we test this issue? What manual steps need to be done to break the functionality? I assume that brutal force like deleting a source field from a database is out of the question.

Michèle’s picture

Patch #12 works for me, too. Thank you very much!
I guess this has something to do with the minimal install profile I've used...

RobbMLewis’s picture

Another core update, and once again patch 12 had to be applied. What do we need to do to get this into the next core update?

phenaproxima’s picture

What do we need to do to get this into the next core update?

For a start, I think an automated test which proves that the bug fix works as intended would go a long way to getting this change in. If anyone is interested in giving that a shot, there is extensive documentation about how to write tests at https://www.drupal.org/docs/automated-testing/phpunit-in-drupal.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
1.3 KB
1.95 KB

Here you go -- a test-only patch to prove that the bug exists, and the same test combined with #12 to prove that it fixes the problem.

The last submitted patch, 26: 3106659-26-FAIL.patch, failed testing. View results

dgaspara’s picture

Works for me. Thanks

timfletcher’s picture

#12 works for me. I was unable to load the Status Report page.

iamweird’s picture

I confirm that #26 fixed the error for me.

marcoscano’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me. Thanks everyone!

danflanagan8’s picture

Status: Reviewed & tested by the community » Needs review

I wrote a horrible update hook like the one below:

function my_module_update_9001() {
  $media_type = MediaType::load('video');
  $field_definition = $media_type->getSource()->getSourceFieldDefinition($media_type);
  $field_storage_definition = $field_definition->getFieldStorageDefinition();
  $field_definition->delete();
  $field_storage_definition->delete();
}

Then I went to /admin/reports/status as got this white screen:

Error: Call to a member function getItemDefinition() on null in media_requirements() (line 138 of core/modules/media/media.install).

Then I applied the PASS patch from #26. The status report page was fixed. However, there were not any warnings about the video media type being messed up. I would expect something like that to be listed in the ENTITY/FIELD DEFINITIONS error perhaps. What is the expected behavior here?

Also when I went to edit the Video media type (the one I mucked up) I got this message in a whitescreen:

Error: Call to a member function getLabel() on null in Drupal\media\MediaSourceBase->buildConfigurationForm() (line 199 of core/modules/media/src/MediaSourceBase.php).

This was at /admin/structure/media/manage/video. This situation might be outside the scope of this issue but it's possible to read this issue as "Missing source field shouldn't cause any whitescreens."

danflanagan8’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs followup

Tagging as "Needs Issue Summary Update" to clarify exactly what is getting fixed here. (Probably just preventing the fatal error.) Tagging as "Needs followup" so that we can work on adding an error to the status page about the missing source field. But that work shouldn't slow down fixing the fatal error.

phenaproxima’s picture

Title: Error: Call to a member function getItemDefinition() on null in media_requirements() » Media types with missing source fields break the status report page
Issue summary: View changes
Issue tags: -DX (Developer Experience), -Needs issue summary update
phenaproxima’s picture

Priority: Major » Normal

Downgrading the issue status, since this is definitely in the "shouldn't occur under normal circumstances" class of bugs.

phenaproxima’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs followup

Filed #3205866: media_requirements() should report missing source fields to add explicit errors to the status report. Therefore, with an improved issue summary and a follow-up for a nicer fix, I'm restoring RTBC. :)

phenaproxima’s picture

Issue summary: View changes

The last submitted patch, 26: 3106659-26-FAIL.patch, failed testing. View results

The last submitted patch, 26: 3106659-26-FAIL.patch, failed testing. View results

  • larowlan committed b2686e3 on 9.2.x
    Issue #3106659 by phenaproxima, aleevas, rajanvalecha12, Drupaldev2013,...

  • larowlan committed 4cea777 on 9.1.x
    Issue #3106659 by phenaproxima, aleevas, rajanvalecha12, Drupaldev2013,...
larowlan’s picture

Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed b2686e3 and pushed to 9.2.x. Thanks!

As there is little risk of disruption, backported this to 9.1.x

Thanks all

🐛🔨

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

loopy1492’s picture

It is still possible to get this error from time to time. In our case, it was because one of the many strange updates to Lightning Media did not get captured in config at some point.

MEDIA: The source field definition for the Remote video media type is missing.

Sadly, the real solution as far as I can tell is to hunt down the config file for the media type and find the source field definition for it. So, for our specific case, I had to hunt down media.type.remote_video.yml and look for the source_configration.source_field in there. Fortunately, it wasn't blank and saw that it was field_media_oembed_video.

I then did a files search for field_media_oembed_video. Of course, it wasn't in my default config folder so I had to go find it in the contrib folders. In the

docroot/modules/contrib/lightning_media/modules/lightning_media_video/config/optional/

folder, I found both the field and the field storage configs:

config/default/field.field.media.remote_video.field_media_oembed_video.yml
config/default/field.storage.media.field_media_oembed_video.yml

Finding the field storage was easy, but I had to kind of hope that remote_video.field_media_oembed_video would work since just plain field_media_oembed_video did not exist anywhere in the code base, contrib or custom.

I imported the config and the errors disappeared. Previously, the field edit page for that media type was also broken and it is no longer broken.