Throw a more detailed error when a target plugin is not found in FeedType::getTargetPlugin()

Hi, thanks for filing a bug report with details!

The AJAX error is quite general (although it looks strange that the code is '200' which means that the request was successful).
Can you check the logs for more details about the error and report these here?

Call to a member function getPluginId() on null in Drupal\feeds\Entity\FeedType->getTargetPlugin() suggests that a configured target plugin could no longer be found. But that the error no longer comes back suggests that the target plugin that was missing is now found again. Unfortunately, the error doesn't tell us which target plugin was missing for a short time, so that makes it very hard to debug. It could perhaps have been an issue with caching. You are sure you didn't install/uninstall modules in between? Or moved around files from a module? Do you have or had modules installed that extend Feeds?

Either way, it would be good if the code checks that the target plugin exists and then throw a more specific error when it doesn't.

Thanks for reporting back. Re-titling this issue.

What errors did Devel cause with Feeds? I remember I did encounter an error as well when trying to print the variables of a feed type with Kint. That could be the same issue.

I agree with #5. That would make sense and help for better debugging. I've just run into "Error: Call to a member function getPluginId() on null". In my case a field had been deleted in the target entity type in the meantime.

The further problem is that the mapping can not be changed anymore due to a different error: admin/structure/feeds/manage/dguard_import_merchants/mapping so that the missing field can not be removed from the feeds mapping.

I solved that by adding the following lines in src/Form/MappingForm.php, Line 158:

      // Remove missing targets (=if the field does not exist anymore)
      if (empty($this->targets[$mapping['target']])) {
        $this->feedType->removeMapping($delta);
        return;
      }

which works and seems to be a practicable solution.

Patch solving the problem with deleted targets described in #9 attached. This doesn't add enhanced error messages for other problems, but in this case makes the mapping usable again and removed the deleted field.

Hm, I don't like that the function is messing with the mapping. We can't be certain that the cause of not finding the target plugin is always gonna be missing a field. It could also be caused by a weird caching issue where somehow a target plugin is not being indexed. Or you when you are working on a target plugin and decide to rename it. Or when you accidentally moved around folders.

Note that the deleted field issue is most likely fixed in dev. With the latest dev version, when you save a feed type, dependencies on fields are calculated and saved on the feed type. This means that when you delete a field, you can see that it will affect a feed type and if you decide to continue deleting the field, Feeds automatically updates the mapping. See #2854914: Deleted Target Field Throws Fatal Error. Come to think of it, all feed types do need to be resaved after updating the Feeds module in order to save these dependency calculations on each feed type. I should be create a follow-up issue to resave all feed types in a database update.

I think throwing an exception is the right thing to do here. To avoid causing a fatal error on the mapping page, the mapping form should catch that exception. It would also be useful to add a functional test here. The test could programmatically add a fake target and then assert that the mapping page still loads correctly. Additionally, the test would try to delete the mapping for the fake target so we can also ensure that an user can clean up.

Would you like to work on that @Anybody? I'd like to make a new release of Feeds next week (or the week after) and I have a few things that are near ready that I like to have finished as well for that release. It would be nice if this thing could then make it too, but not sure if I'll have enough time to get to it.

Hi MegaChriz,
I completely agree with #11. Currently I don't have the time to care for that, sorry. I guess you also don't. So it would be nice if someone may pick this up.

Thank you all so far!

I was still experiencing this issue:
MegaChriz's point about re-saving each item is spot on:
Come to think of it, all feed types do need to be resaved after updating the Feeds module in order to save these dependency calculations on each feed type. I should be create a follow-up issue to resave all feed types in a database update.

I had problems with a couple of feeds with the mappings of some fields being missing. Updating these and then resaving allowed the update to complete.

@paulgranted
Resaving all feed types was in fact included as a database update since 8.x-3.0-alpha2.

Tagging issue for Global Contrib Weekend Event at SFDUG. Thursday Jan 23.

Also updated issue summary.

I ran into this error while working on #2989279: Add a target to entity ID and then switching branches (on the Feeds git repo).

This patch prevents the fatal error in the following way:

• It adds an exception class called "MissingTargetException".
• It adds a placeholder target definition class called "MissingTargetDefinition".
• The mapping form code checks if the target definition exists and if not, it creates a placeholder target definition of type MissingTargetDefinition.
• CSS is added to highlight the mapping row that has got errors.

See the image for how this looks like in the UI:

Thank you @MegaChriz, I'll review that soon! :)

Sorry, finally I was able to review and test this. It works great! :) Wonderful work once again, you are wonderful.
RTBC!

@Anybody
Thank you very much for your review and your positive words. :)

Committed #16

Hm, recent changes introduced new failures, apparently.

Patch to fix issues.

Committed #22.

Great work, thank you very very much @MegaChriz!