Problem/Motivation
The following error has been reported multiple times:
Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, NULL given
The only potential source of this error is in Drupal\jsonapi\ResourceType\ResourceTypeRepository::getRelatableResourceTypesFromFieldDefinition()
:
$has_target_bundles = isset($handler_settings['target_bundles']) && !empty($handler_settings['target_bundles']);
$target_bundles = $has_target_bundles ?
$handler_settings['target_bundles']
: $this->getAllBundlesForEntityType($entity_type_id);
return array_map(function ($target_bundle) use ($entity_type_id, $resource_types) {
$type_name = "$entity_type_id--$target_bundle";
return isset($resource_types[$type_name]) ? $resource_types[$type_name] : NULL;
}, $target_bundles);
The two known reasons that NULL
might be generated is if an entity reference field has handler settings referencing a missing target bundle or the resource type name has been overridden by JSON:API Extras.
Proposed resolution
Neither of these reasons is a bug in JSON:API, per se. Instead, they could be caused by bad configuration, a poorly defined custom base field, or by JSON:API Extras.
In order to help users suffering from those bugs, introduce exceptions that will pinpoint the source of the errors.
User interface changes
Nope.
API changes
Nope.
Data model changes
Nope.
Release notes snippet
N/a
Comment | File | Size | Author |
---|---|---|---|
#203 | interdiff-124-201.txt | 1.66 KB | simohell |
#203 | interdiff-124-198.txt | 1.43 KB | simohell |
#201 | 2996114-201.patch | 19.78 KB | bsains |
#198 | 2996114-198.patch | 19.76 KB | simohell |
#190 | 2996114-190.patch | 14.92 KB | jungle |
Issue fork drupal-2996114
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
lawxen CreditAttribution: lawxen at Sparkpad commentedReproduced steps:
The commerce_order's billing_profile filed reference profile-customer, but the profile bundle:customer has been deleted by us, so the error come.
But I don't think we need spend time on it.
Comment #3
Wim LeersComment #4
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedHow did you find what was deleted?
I am trying to debug and I cannot find what was deleted or not to make this happen.
I think there should be a check placed in here that will at least write to logs what is passing in a NULL resource
Comment #5
Wim Leers@SocialNicheGuru This debug patch should help you figure it out :)
Comment #6
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedThanks for this.
drush pm-uninstall jsonapi
The following extensions will be uninstalled: entity_share, entity_share_client, entity_share_server
Do you really want to continue? (y/n): y
array(1) {
["tags"]=>
NULL
}
I had to add a vocab called 'tags'.
Comment #8
ARUN AK CreditAttribution: ARUN AK commentedI am also facing the same issue after upgrade into the latest version.
Comment #9
ARUN AK CreditAttribution: ARUN AK commentedCan we add a check here to avoid this error?
Comment #10
malks CreditAttribution: malks commentedIs there anything more refined suggested to find the missing bundle? We have a complex site where a clean install doesn't work... I put the debug patch in place and so far the
var_dump
is 6.1G :/. I've tried running this under breakpoint debugging but I can't seem to get it to hit the right place to inspect variables. Any better ideas or a way to make this more robust?Comment #11
malks CreditAttribution: malks commentedActually, the snippet below just before the call to
array_reduce
pinpointed it for me. Given that it's just trying to remove invalid route candidates this should just silently move on to the next target as the previous commenter suggested?Comment #12
jungleReproduced, when upgrading Drupal core to 8.7.0-beta1, Move it to Drupal core,
Comment #13
jungleFixed the error, and using
foreach
, instead ofarray_reduce
, to return early if possible.Comment #14
jungleHi @ARUN AK
Can not add type hint `ResourceType` to the 2nd argument here, it could be null, null is not an instance of ResourceType, that's why it throws errors.
Simply remove the type hint, your patch works.
Comment #15
jungleComment #16
ARUN AK CreditAttribution: ARUN AK commentedI have combined the changes from @jungle's patch and made it work with 8.6.x.
Comment #17
tengokuHi All.. i'm uploading this to re-roll the patch for drupal core on 8.7.2, since the patch posted here looks like is for the contributed module and not the core one.
thanks!
Comment #18
egruel CreditAttribution: egruel commentedThe patch in #17 Work for me on drupal 8.7.2
This issue need to be reopen.
Comment #19
grahlCan reproduce this when installing a site from configuration as well with 8.7.2, patch #17 resolves it.
+1 on reopen (or creating a new issue if the maintainer isn't watching here).
Comment #20
bdlangton CreditAttribution: bdlangton commentedPatch #17 resolves the issue for me as well, and I agree with re-opening.
Comment #21
kyletaylored CreditAttribution: kyletaylored at FFW commentedHad the same issue, #17 worked for me against 8.7.3.
Comment #22
nielsaers CreditAttribution: nielsaers as a volunteer and at Dropsolid for Dropsolid commentedSame here, #17 worked for me against 8.7.2
Comment #23
mikemadison CreditAttribution: mikemadison at Acquia commentedit looks like this issue auto-closed without activity and didn't get merged in, can we get it reopened?
Comment #24
tim.plunkettComment #25
lcatlett CreditAttribution: lcatlett commentedThe patch in #17 resolves this issue for me against 8.7.3
Comment #26
gnikolovskiPatch #17 resolves this issue for me.
Comment #27
bradjones1Comment #28
Wim LeersCommitting #17 would be accepting broken configuration. That'd be wrong.
We need to refactor #5 to detect the problem and provide an actionable error response.
Comment #29
Wim LeersThe number of people with invalid configuration is frightening by the way 😨
Comment #30
Wim LeersComment #31
tengokuHi All... based on #29 comment, then i wrote this debug patch. instead of using the function the triggered the error, i put a try/catch code on the parent functions. Showing the entity and the path affected.
Instead of breaking the site, this just send an error message (this error is only trigger on clearing cache - on my experience - ).
For testing it, I did a clean install and just enabling the modules, later a config export/import.. and my error(s) is this one.
JSONAPI: Fatal error looking internal resource types on entity commerce_payment_method, bundle credit_card, field billing_profile and path /commerce_payment_method/credit_card
JSONAPI: Fatal error looking internal file target resource types on entity commerce_payment_method, carry credit_card and path /commerce_payment_method/credit_card
JSONAPI: Fatal error looking internal resource types on entity commerce_payment_method, bundle paypal, field billing_profile and path /commerce_payment_method/paypal
JSONAPI: Fatal error looking internal file target resource types on entity commerce_payment_method, carry paypal and path /commerce_payment_method/paypal
Now, i'm just an ignorant on the error, what this means and what i have to check? at least i know is the commerce_payment_method module that is giving me problems
Comment #32
ivan.chavarro CreditAttribution: ivan.chavarro at Globant commentedHi everyone, the patch #31 solves the issue, the errors are displayed before you run drush cim, but after they go away.
Comment #33
gabesulliceI think I found a legitimate source of this bug (without invalid configuration). I described that problem in #3034786-37: ResourceIdentifier::getVirtualOrMissingResourceIdentifier() ignores field aliases; causes $relatable_resource_types field to be empty and results in an error. Transcribing here:
Restoring the original title since it's now clear that the problem is bigger than the prescribed solution.
Comment #34
joachim CreditAttribution: joachim commentedThe issue summary needs an update, as I can't really tell what this issue's proposed resolution is.
This former title looks like one possibility, for instance:
> Detect invalid entity reference field configuration (pointing to non-existing bundles) and provide an actionable error
Comment #35
joachim CreditAttribution: joachim commentedCatching a TypeError seems like a pretty hacky way to do this. Code should be checking we get something legitimate earlier on rather than having PHP complain.
Comment #36
nishantkumar155 CreditAttribution: nishantkumar155 as a volunteer and at Cognizant Technology Solutions commentedHello drupaler ,
I have just applied patch #31 it not worked for me for 8.7.7
Comment #37
penyaskitoHad this same problem myself, #17 fixed it for me.
In my case, the problem happened with a custom module adding an entity reference to commerce_order in hook_entity_base_field_info:
If I removed that line, it worked. With #17, it works.
Comment #38
danrodPatch #16 Works for me (Running 8.6.x), thanks a lot @ARUN_AK for your work on this, I am having huge issues with this JSON API module (which I'm planning to uninstall in the near future), and this patch is a temporary fix for me.
Comment #39
mglaman#37, @penyaskito so we might be able to reproduce by adding an entity reference base field via hook_entity_base_field_info to a test entity against a node type, with handler_settings and target_bundles defined?
I think one step is to get a failing test.
Comment #40
saltednutThe site builders among us often have no idea when or why these things happen and its typically contrib modules that lead to this. We shouldn't judge too harshly... 😇
Re: #35 - Is there something that we can try to do to inform users without compromising the integrity of the API via hacks?
For what its worth, I can confirm that the patch from #31 obfuscated the error we were seeing during installation, but I'm still at a loss for what configuration in my system was/is invalid. I don't see anything in the logs about it after applying the patch even after re-installing everything from our profile down to the module that included jsonapi.
While it isn't the job of jsonapi to be pointing these things out, it probably IS the job of core to disallow invalid configuration to be installed. We shouldn't be getting down to a level with jsonapi failing during install due to invalid config already installed. However, its better to find/discover this stuff via jsonapi install than it would be during production. Unfortunately, it looks like at this time, all we are able to do is gracefully walk around the error while remaining ignorant as to what the original source is.
Comment #41
gabesulliceHi everyone! Thank you all for your help and interest on this issue! I see that this issue has 36 followers, so I presume there are at least that many people on the internet that are experiencing a similar error. Unfortunately, the patches provided cannot be committed because I do not feel comfortable with them and we don't yet have a "complete" issue for a core commit. Why?
We need a reset.
In #33, I detailed one possible source of the error. I propose that we write a test that proves that bug, then write a patch that fixes it, then commit it.
I suspect that it will resolve at least some of the errors that the followers of this issue are experiencing.
I also suspect that it will not resolve it for others. If you are one of the followers of this issue and that committed patch does not solve your error, we really need you to spend the time to provide steps to reproduce your error and share them with us so that we can write a test and a fix for your issue. Your help will be tremendously appreciated! AND, you'll get contribution credit!
Comment #42
gabesulliceignore
Comment #43
gabesulliceNit: "A resource type with the name $type_name does not exist."
Comment #44
gabesulliceThis patch should help anyone identify the source of this error. It does not provide a fix on its own because the exceptions that are thrown must either be a result of bad configuration, a module defining a bad base field, or JSON:API Extras.
Please apply this patch, test it, and report the results! If the patch helped you find and fix the source of your problem, we would still like to know, so thank you for reporting that in advance 🙏
Comment #45
effulgentsia CreditAttribution: effulgentsia at Acquia commentedFor adding test coverage to #44, I think we can follow the lead of #37, and do something like:
Except maybe using test entity types instead of users and nodes. But I can confirm that if I manually enable the above module on my machine, and delete the Article content type, then when I clear caches, in HEAD I get the same error that's in the issue summary, and with #44 I get the new exception.
Comment #46
effulgentsia CreditAttribution: effulgentsia at Acquia commentedWe might want to open a separate issue to throw an exception in
EntityFieldManager
orEntityReferenceItem
when encountering #45.Comment #48
gabesulliceThe #47 failure was just a testbot thing. I retested #44 and it came back green.
Anyway, here's something similar to @effulgentsia's suggestion in #45. Indeed, the test only patch gives the reported error. The complete patch should give one of the new LogicExceptions. The interdiff is just me cleaning up and rewording that exception.
Comment #50
mglamanChiming in that I have found a way to immediately reproduce this using Drupal Commerce 2.14
I have JSON:API module installed. I then installed commerce, commerce_order, commerce_price, commerce_store and then 💥
I haven't determined if it's due solely to base fields or field config.
---
When Commerce is installed first, everything is OK. I think that is due to the fact we install an entity reference field config definition on installation.
This is fixed in Commerce on #3002939: Convert order/product multivalue configurable fields back into base fields.
So it seems to only affect field config definitions. Given: https://git.drupalcode.org/project/commerce/blob/8.x-2.x/modules/order/c..., there is to target bundles.
Comment #51
gabesullice@mglaman, the patch in #48 adds more informative exceptions that should be useful to you. They should point you in the right direction. Since you now have reproduction steps, if #48 helps you find the root cause of the bug in Commerce, then I think it'd be safe to RTBC this issue. If you find out that the bug is still present and the exception is unhelpful, understanding why would move this issue forward even further :)
Comment #52
penyaskito@gabesullice @mglaman Sorry I couldn't get back to this yet. #50 may actually explain my issue, as I was using the said modules. I'll try the commerce patch explained at #50, if the error disappears for me there's no need to write the test described at #37 as it won't be JSON:API fault.
Comment #53
mglaman🙌this patch helps isolate the error, which makes for a much nicer developer experience over the cryptic generic message.
Unrelated to this issue, it is uncovering a weird problem on install. The profile type bundle "customer" is not being recognized, even though a dependency installed it.
:/ or it's due to this all running from
ConfigImporterBatch
So it seems like this bug could occur if routes are rebuilt before config sync is finished and bundles are installed completely.
Final edit. When I applied #3086307: Improve installer performance by ~20% by rebuilding the router after the entire installation is complete rather than after each module the problem fixed itself. JSON:API routes are rebuilt too often while the system is in flux and not in a stable state.
Comment #54
larowlanAre we sure an exception is the best way to handle these - i.e. can we not just log the missing bundles and remove them?
Whilst an exception is more helpful than the current state, it still results in a 500 error that a site builder may be unable to resolve.
Comment #55
mglamanWould a site builder then know there's a problem? How would we bubble it up? With silent logging to the database I wouldn't know there was an edge case error with my route compilation
The real bug is Drupal not ensuring the setting exists on the field config definition.
Comment #56
larowlanViews just logs an error in a similar scenario
Comment #57
gabesullice@larwolan, these exception are only thrown during a router rebuild and only after a config change or a module install. I think this will appear during a development phase when it's clear what needs to happen.
FWIW, I'm not aware of any other place where JSON:API logs errors to the watchdog. Everything either fails during dev or results in HTTP error responses.
Comment #58
gabesulliceWait, I think I see what you're saying now. If a site builder enables the module or somehow breaks config, they're gonna be stuck with a WSOD from this exception and won't be unable to fix the problem. Let me do some digging to confirm whether that's actually the case. I'd like to avoid watchdog if at all possible. That's where errors go to die.
Comment #59
bradjones1🔥️
Comment #60
ndobromirov CreditAttribution: ndobromirov at FFW commentedI've tested the patch in #48 by manually porting it to the current contrib 2.x version for the module.
The error I am experience in #3089393: Exception in JSON:API fetching content. is not resolved with the patch from #48. I am still getting the same exception and not the new one as introduced here.
Note that the patch in the referenced issue solved it for now. Should it get incorporated here or I should reopen the other issue?
Comment #61
gabesullice@ndobromirov, I think you should reopen the other issue since you're testing with 2.x contrib JSON:API, not core's JSON:API. Perhaps something has diverged.
Comment #62
Wim Leers#41++
#50 and #53 confirm the value of this patch :)
Comment #63
Morbus IffI've running into the same "Argument 2" error after adding jsonapi and jsonapi_extras to my installation profile in an 8.7.x site. Using a modified version of #48, I receive "A resource type with the name contact_form--contact_form does not exist." My installation profile does require the contact module, as well as provides configuration in config/contact.form.feedback.yml and config/contact.settings.yml. This particular break was a bit surprising to me, though, as I was expecting to see the same thing as mglaman, as this installation profile also installs commerce and all the fixins.
Unlike mglaman's comment in #53, however, an 8.7.x version of #3086307: Improve installer performance by ~20% by rebuilding the router after the entire installation is complete rather than after each module didn't help - the same Argument 2 error occurs. In this particular case, the installation died with "A resource type with the name block_content_type--block_content_type does not exist."
I also haven't had any luck with moving the installation of jsonapi to "later" in the install profile. My profile has two module installation points - the standard one that happens via .info.yml (which I use for all core and contrib modules) and then a second one, as a custom install task later (which is used for all custom modules). Moving jsonapi to the second custom install task results in "A resource type with the name block_content_type--block_content_type does not exist."
Sadly, #17 is the only patch that seems to work for me so far. It allows me to enable jsonapi and jsonapi_extras in the profile's .info.yml, allows me to ship content/jsonapi*.settings.yml in the profile, and everything "seems" to work so far. #17 "accepts" the missing (as opposed to broken) configuration that occurs at installation time in between all the dependencies, refreshing, rebuilding, etc., etc.
Comment #64
gabesulliceAFAICT, the exception
can only be encountered when JSON:API Extras has changed the resource type name. For example, maybe you have renamedcontact_form--contact_form
tocontactForm
(or something else)? If that's the case, can you either try removing those renames or disabling JSON:API Extras? If that fixes it, then we should open a bug report for your problem in the JSON:API Extras issue queue.Comment #65
Morbus Iff@gabesullice: Retested. With *just* jsonapi in my 8.7.8's installation profile (no jsonapi_extras, no config/install/jsonapi.settings.yml), I receive "A resource type with the name contact_form--contact_form does not exist.".
Comment #66
gabesulliceThanks for doing the extra testing @Morbus Iff! I think it's clear that there's an unmet config dependency problem then. Let me try to adjust #48 to give us even more clarity.
Comment #67
gabesulliceI'm really quite stumped. This is an enhancement to #48. Hopefully it'll help us narrow down source of the problems even further.
Comment #68
gabesullice🙈
Comment #69
lawxen CreditAttribution: lawxen at Sparkpad commentedOne year ago this is a suffer problem when I create this issue.
I'm writing Drupal code just now, and I met this crash error again when clear the cache in other project.
After investigate, I found this crash "help me a great favor". I like this crash.
Below image is my code.
I create a group content plugin for entity type: pipeline, but in the GroupPipelineDeriver I didn't change the ProductType to PipelineType when copy code from other place. In my project ProductType and PipelineType has same bundle names.
So if there's no jsonapi, no this crash, I won't find this error until several months or several years has past, but after so long time, I guess the system's data has been in chaos.
So just give more detailed crash info(later patch) may be better than make the error disappear(earlier patch).
Comment #70
gabesullice👋 hi @lawxen! It's good to see you in the issue queue again! I'm glad that the new patch helped you find a problem :D That's what it was for.
Comment #71
gabesulliceI just had a very productive debugging session with @Morbus Iff in Slack.
We finally found out the source of the issue and why I was so stumped in #67.
Drupal 8.7 is broken and will not be fixed because the release cycle for 8.7 has ended. The solution is to upgrade to 8.8 or above.
If you are on Drupal 8.8 or above and you are still getting an error, then please apply #68. Just like @mglaman and @lawxen have confirmed, it will be very useful for finding the root cause of the error outside of JSON:API. Like @lawxen said, it is better to fix your site rather than ignore the underlying problem (which is what #17 does).
For background, the reason that 8.7 is broken but 8.8 is not is because #3018287: ResourceTypeRepository computes ResourceType value objects on *every request* unknowingly fixed one source of the now infamous
Argument 2
error. Since #3018287 was a performance enhancement, it was not backported to 8.7.Since I have been looking exclusively at the 8.8 and 8.9 branches, I could not understand why @Morbus Iff and others were having so many problems.
So, the solution is to upgrade to 8.8. If you are still having issues after upgrading to Drupal core 8.8, apply the latest patch on this issue. The exceptions should help you identify the source(s) of the problems in your sites custom/contrib modules or in Drupal core itself. If you find them, please post back here and open separate issues to fix them. I also believe that #3086307: Improve installer performance by ~20% by rebuilding the router after the entire installation is complete rather than after each module should be very useful for working around configuration dependency issues.
Comment #72
gabesulliceGiving credit to @Mobus Iff
Comment #73
gabesulliceSee #71 for details about how you can find and fix the source of the
Argument 2 passed to...
error, if you're encountering it.Comment #74
lawxen CreditAttribution: lawxen at Sparkpad commented@gabesullice Really thanks for your patch, I indeed disappeared for a long time, haha. But I found some weird thing: "The crash not always happen when system has same problem" , maybe some other deeper problems is in drupal core, so reported it out.
Steps:
Environment: Drupal 8.8.x-dev, group:1.x-dev
Uninstall these group content plugin in group config page. then execute drush cr, then change my code to the bad version of step 1. install these plugin in group config page again, execute drush cr, no error appeared
It seems that drupal core stores some route info into the database and didn't change when redo sth......
Comment #75
lawxen CreditAttribution: lawxen at Sparkpad commented@gabesullice Suggest that #68 be committed to drupal core, or it's confused when people met this incomprehensible crash after enable jsonapi.
#68 make people think that Drupal is robust💪, it's not a buggy system🐛.
So I reopen this issue. feel free to close it again 😁
Comment #76
lawxen CreditAttribution: lawxen at Sparkpad commentedComment #77
lawxen CreditAttribution: lawxen at Sparkpad commentedComment #78
gabesulliceFair enough @lawxen. The reason I changed it to a support request and marked it "Fixed" is because the issue really isn't really a bug in JSON:API. I guess we could make this issue a "Task" again.
I'm not really interested in converting the exceptions into log messages like @larowlan suggested in #54 and #56. Maybe there's another approach or maybe we can simply make the case that exceptions are really the best choice.
I'm curious what @Wim Leers thinks. I pinged him on Slack.
Comment #79
Wim LeersThe only other case of using logging in JSON:API is
\Drupal\jsonapi\EventSubscriber\ResourceResponseValidator::validateSchema()
.Everywhere else, we throw
\LogicException
s. For example:which is also an explicit data modeling issue, just like this.
Generally speaking, we throw exceptions when there is no way that we can deliver a sensible response. That's the case here. If we'd remove the missing bundles like @larowlan suggests, other data integrity problems may arise, and JSON:API should not allow for that.
Comment #80
gabesullice@Wim Leers, I should have also pointed you to #58 when I asked for your opinion. It was poorly written, but the gist is that if a site builder goes to the "Extend" page and enables JSON:API and they have a pre-existing fault in their Drupal site that would trigger our exception then they're going to get a WSOD after enabling JSON:API. Since the exception will be thrown during a router rebuild, I don't think they'll be able to get back to the "Uninstall" page where they could remove JSON:API (since no routes will exist).
Just writing this out, I had the idea that we could wrap the code in
Drupal\jsonapi\Routing\Routes::routes()
with a try/catch block. If we catch an exception, we won't generate any JSON:API routes. In that case, we could somehow get the exception into the face of the user. Mabe via drupal_set_message or w/e it is now.Comment #81
Wim Leers#80 sounds sensible. I think that means this patch needs work though?
Comment #82
gabesulliceYep.
Comment #83
gabesulliceSo, to summarize what this patch needs:
Drupal\jsonapi\Routing::routes()
causes an unrecoverable WSOD if you install JSON:API (this can be manually validated by just modifying the code and trying it locally)Drupal\jsonapi\Routing::routes()
and should be surfaced to the user without causing a WSOD. Ideally, that message will appear whether you enable JSON:API in the admin UI or via drushVolunteers needed ;)
Comment #84
Morbus IffSadly, I've been working on upgrading to 8.8.0-beta1 today, and can confirm that "out of the box", 8.8.0 does not fix this error that plagued me in 8.7.x (see #63 and the outcome #71). I'll have to apply the diagnostic patch (#68) again and see what I can see :(
Comment #85
nishantkumar155 CreditAttribution: nishantkumar155 as a volunteer and at Cognizant Technology Solutions commentedHi All.. i'm uploading #17 patch for drupal core on 8.7.10, since the patch posted here looks like there is line no change
thanks!
Comment #87
BR0kENIs this how it supposed to be?
Comment #88
BR0kENTBH, it takes some extra time to understand this code. Not a quick read.
Comment #89
BR0kENHere's another point of view on this issue. I think we should use internal resource names under the hood all the time and expose their renamed versions only to the clients (and decode as early as possible to internal ones when a request comes to the backend (POST, PATCH, etc)).
Comment #90
BR0kENA slightly different version that allows further use of
getTypeNameInternal()
.My bad: forgot the
*.txt
extension for the interdiff.Comment #91
BR0kENComment #92
Morbus IffComing back to this after a vacation. To recap: out of the box (no JSON:API patches), 8.8.0-rc1 does not fix the error that plagued me in 8.7.x (see #63 and the outcome #71). This morning, I applied patch #68 to provide more analysis of the issue, and it dies with an error against Commerce 2.15: "Invalid field settings detected. The `billing_profile` field on the `commerce_order` entity type has one or more missing target bundles: `customer`".
This is the same error reported in #53 by @mglaman, however, applying #3076307.12 DOES NOT improve things. Profile installation still dies with the same error.
Comment #93
BR0kEN- The patch that proves the error exists when the
typeName
is modified (as JSON:API Extras does).- The patch that provides the fix.
Comment #94
BR0kENHowever, this fix is pure garbage in core as we satisfy the
JSON:API Extras
only. I'm thinking about deprecating the ability to modifytypeName
inDrupal\jsonapi_extras\ResourceType\ConfigurableResourceType::setJsonapiResourceConfig()
.Comment #95
BR0kEN... and my patch breaks
paramconverter.jsonapi.resource_type
.Comment #97
BR0kENThis one seems better as it allows using both internal and renamed ID of a resource type.
Comment #99
BR0kENComment #101
Morbus IffI believe I've got a reproducible, smaller, use-case for this issue.
composer require "drupal/admin_toolbar"
composer require "drupal/commerce"
core/profiles/demo_umami/demo_umami.info.yml
file to include the following after thecontent_translation
dependency:drush -y site-install demo_umami --db-url="sqlite://localhost//tmp/site.sqlite" --account-name="admin" --account-pass="admin" --account-mail="admin@example.com" --site-mail="admin@example.com" --uri="localhost:8888" --verbose --debug
.Invalid field settings detected. The `billing_profile` field on the `commerce_order` entity type has one or more missing target bundles: `customer`
. If you print instead of throw that error, you'll THEN seeA resource type with the name `profile--customer` does not exist. Ensure that the `billing_profile` field on the `commerce_order` entity type has declared all of its dependencies, either via config or the providing module's info file.
.Comment #102
mglaman#101 came to the same conclusion I had in #53, but my fix did not apply for @Morbus Iff.
I think this issue deserves to go back to RTBC as it helps track down the problem. The issue we're experiencing is a race condition on route rebuilds and config import. Where there is a config entity bundle not yet present for a base field.
Comment #103
g_miric CreditAttribution: g_miric at FFW, Develomon commentedHi all,
Here is my case and how to resolve it:
I used JSON:API Extras to override resources and after that, I exported related configs to a custom module.
After updating the core to 8.8.0 I got the error.
There were 2 issues:
1. I overrode "menu_link", and after update the resource type was changed to "menu_link_content" and I needed to update that config.
2. I have content entities with no bundles, and when I overrode the and exported I got something like this
and I needed to update the config to
after that it was working fine.
Comment #104
BR0kEN@g_miric, because
jsonapi
doesn't work with renamed resource types. #99 fixes this.Comment #105
oeklesund CreditAttribution: oeklesund commentedUpdating from 8.7.10 to 8.8.0, I ran into this issue.
I can confirm that applying both patches below solved it for me:
- #99
- #3086307: Improve installer performance by ~20% by rebuilding the router after the entire installation is complete rather than after each module
Comment #106
Rajab Natshah CreditAttribution: Rajab Natshah at Vardot commentedPatch #99 is working
Thank you
Comment #107
damontgomery CreditAttribution: damontgomery at Palantir.net commentedPath #99 worked for us as well with a renamed resource type from jsonapi_extras.
Comment #108
hkirsman CreditAttribution: hkirsman commentedThis happens for me after adding #99 and as described in #105
Comment #109
hodba CreditAttribution: hodba commentedThe same error of #108 happened with me as well. Drupal 8.8.1 with Lightning profile enabled
Comment #110
BR0kEN#108: what is the resource that cannot be located?
Comment #111
amen CreditAttribution: amen commentedI'm on 8.8.1.
Applying the patches from #99 and the one from #3086307, as described in #105, did not solve this for me.
Comment #112
BR0kENIt would be awesome if you could provide the additional information to help to resolve this faster.
Saying "it's not working" doesn't help at all.
@amen, what error do you have? Is it the same as in #108? Can you find the exception with
Unable to locate the resource type neither by the internal or public name.
, put this code before and do the steps to get the error again?Comment #113
robin.ingelbrecht CreditAttribution: robin.ingelbrecht at EntityOne commentedI agree with @BR0kEN. Can we add the additional info in the runtime exception?
Comment #114
ronaldmulero CreditAttribution: ronaldmulero at U.Group commentedPatch #99 allowed me to upgrade a JSON Extras 8.x-3.13 enabled site from core 8.7.11 to 8.8.2. Thank you.
Comment #115
vgardnerGetting the same error as #108 after applying patch #99. Running on Drupal 8.8.2. Happens consistently even after reinstalling the jsonapi module.
Comment #116
BR0kEN@vgardner, any chance you can do what's described in #112?
Comment #117
lamp5@vgardner
reinstall or re enabling ? Because jsonapi is in core.
Comment #118
BR0kEN@lamp5, Drupal no longer use
enable/disable
words. Even theExtend
page saysInstall
. Moreover, I doubt the patch can be applied to contrib.Comment #119
vgardnerThanks for the quick response @BR0kEN. I've managed to fix the issue by using the debug info output by following your comment on #112.
Context:
The D8 site I was trying to install Json API on had been migrated from a D7 site. It turns out there were two entities that hadn't been created during the migration, a Content type and a Paragraph type. So the problem was solved after I created those two manually with the exact machine names being output by debugging info from #112 (plus applying patch #99).
Steps:
1. Installed Json:API (without patch #99)
2. Applied patch #99.
3. Included debugging info from #112.
4. Ran
drush cr
, received output stating that "paragraph" and "machine_name_of_paragraph_type" could not be located.5. Created the missing paragraph manually (with exact machine name).
6. Ran
drush cr
, received with output stating that "node" and "machine_name_of_content_type" could not be located.7. Created missing content type manually (with exact machine name).
8. Ran
drush cr
, all worked ok.9. Ran
drush pm-uninstall jsonapi
10. Ran
drush en jsonapi
. Json API worked without any errors.Also, FYI I tried a clean install of Drupal 8.8.2, I did not encounter this issue at all. So I'm guessing there's a case during migration where if certain entities are not created properly, we might run into this exception. I'm happy to provide any other info.
Comment #120
BR0kENWow, thanks for your input!
Might be the problem exist. I see one possible scenario:
- create the content type;
- clear cache;
- remove content type;
- clear cache.
There's a little chance that on the last step the ResourceTypeRepository will use cached entity definitions where staled content type will be present and for which resource creation won't be possible. Need to test that.
Comment #121
BR0kENI was unable to reproduce. I'm wondering how I can get stale entity type definitions that exist in cache only (#119 proves this).
Comment #122
BR0kENI guess I got it. We take entity type and bundle from field settings. Those entity types and bundles may easily be missing. I'll try to provide a test.
Comment #123
BR0kENThis patch should fix the issue and inform about broken configurations.
Comment #124
BR0kENAnd the proving test case.
The previous implementation was trying to create resource types based on entity type ID and bundle(s) from field configuration. In case the entity type/bundle no longer existed the exception has been thrown. The new patch will produce a warning that notifies about a broken configuration that must be amended.
Comment #125
BR0kENComment #126
arefen CreditAttribution: arefen commentedPatch #124 fixed my problem
Comment #127
super_romeo CreditAttribution: super_romeo as a volunteer commentedPatch #124 suit for D8.8.3 also.
Comment #128
hanoiiI got here because of this error as well as another fatal error:
I am confused about the issue here. The patch at #124 gave me the following errors:
Now, paragraph:x definitely exists and yes, it was renamed to something el through jsonapi_extras. Although it was mentioned here, the error implied the entity was not there, not that it was renamed or anything which would make more sense if the idea of this if to pinpoint issues.
I sort of understand this is an issue with jsonapi_extras, but what does this mean? Shouldn't we be able to rename the url or resource type through jsonapi_extra or is there something else jsonapi_extra should be doing to cope with this?
The fatal error was a nasty one to find, and also jsonapi_extra to blame, but unrelated to this issue.
Comment #129
hanoiiI think that #3106297: JSON:API (Core) 8.8x is incompatible with jsonapi_extras config is probably the issue reported on the jsonapi_extras side.
Comment #130
BR0kENThe things you are describing are weird enough. Am I correct assuming this is your entity structure?
The above code from #124 takes care of locating a resource by internal ID (that is not renamed by JSON:API Extras). If the resource was renamed, the below logic starts a lookup by comparing entity type ID & bundle across all available resources you have. Only if these two lookups haven't found the resource the warning will bubble.
Could you please provide a real configuration so I am able to help? Right now I think there is an error on your side rather than something wrong with #124.
Offtopic: I've replicated the needed functionality (fields rename, entity types disabling and adding includes by default) of JSON:API Extras by a single event subscriber. This was done not of a good life: the last (almost) two years
jsonapi_extras
loses the race tojsonapi
that is developed more actively.Comment #131
hanoiiYes, that is the correct entity structure.
Hmm, I'd have to try debugging this if I have more time, however the "Resource type" was also renamed from `paragraph--x` to `paragraph--item` as well as the path, so I wonder if that has anything to do with that?
What configuration are you looking for? In reality the only thing changed was paragraph--x to paragraph--item and the path from /paragraph/x to /paragraph/item
Comment #132
BR0kENWhat do you have in
target_bundles
forfield.field.node.faq.field_paragraphs.yml
?Comment #133
hanoiitarget_bundles: null
I should also add that disabling jsonapi_extras prevents this warnings from appearing.
Comment #134
BR0kENCould you please run the following Drush commands and paste the output?
drush ev "var_dump(\Drupal::service('entity_field.manager')->getFieldDefinitions('node', 'faq')['field_paragraphs']->toArray())"
drush ev "var_dump(\Drupal::service('entity_field.manager')->getFieldDefinitions('node', 'faq')['field_paragraphs']->getItemDefinition()->getSettings())"
drush ev "var_dump(\Drupal::service('entity_type.bundle.info')->getBundleInfo(\Drupal::service('entity_field.manager')->getFieldDefinitions('node', 'faq')['field_paragraphs']->getItemDefinition()->getSetting('target_type')))"
Comment #135
hanoii@BR0kEN sorry for the delay:
See:
- https://pastebin.com/8ZVZna1d
- https://pastebin.com/Vqi2RKA3
- https://pastebin.com/rB2gZk6S
Comment #136
BR0kENIs our
x
paragraph's bundle present among keys at https://pastebin.com/rB2gZk6S?Comment #137
hanoii@BR0kEN it is, I edited the pastebins to reflect that. Sorry about the name changing btw. At first I did it because of an NDA and just in case but then I got consent but now to keep it with context I edited the pastebins.
Not sure if this is an indication fo anything, but:
- The entity does exist
- I tried removing it from the bundles and importing the configs and the warnings were still there
- Disabling jsonapi_extras prevents the warnings from showing up even though paragraph
x
is thereComment #138
BR0kEN- I made a clean install of Drupal 8.8.5, JSON:API Extras 3.14.0 and Paragraphs 1.11.0.
- Created the
x
paragraph with thefield_title
text field.- Created the content type with the
field_paragraphs
that includes all paragraph types.- Did override the
paragraph--x
JSON:API resource type tox
and path tox
.- Rebuilt the cache.
After these steps I got no errors. Also, in #128 I see the
[error] withRelatableResourceTypes() must be called before getting relatable resource types.
which may indicate you have some additional customizations of thejsonapi
ecosystem.Comment #139
BR0kENHere are the results in accordance to #134:
Comment #141
BR0kEN9.0.x re-roll.
Comment #142
bbrala@BR0kEN i've used your changes to see if i could fix JSON:API extra's, tests there seem to pass, but perhaps you can have a look there also?
See https://www.drupal.org/project/jsonapi_extras/issues/3069220
Comment #143
eelkeblokWe are running into this error and I think the reason is that config dependencies are not making sure everything is in place before the routes are rebuilt;
I am getting failures on a few Paragraph types that were added recently and are *not* present in the database I am running a deployment against. I am getting this error because the resources for those paragraph types in the array passed to Routes::hasNonInternalTargetResourceTypes() has NULL values, presumably because those paragraph types don't actually exist, eventhough the "parent" resource is referring to them.
Of course, if the config import were able to complete, everything would be fine, but we are currently in a catch 22 situation where the config import can't complete because halfway through an error is thrown because JSON:API is trying to rebuild routes for a resource that is essentially still incomplete. This is in core 8.8.8 with jsonapi_extras.
Edit: I now worked around the issue by importing the two offending paragraph types in an update hook (which ensures they are available before the jsonapi_extras resource is imported), which seems to do the trick. I guess this means this is an issue in jsonapi_extras, that should list field configurations as dependencies to a resource definition (this would in turn ensure the paragraph types get imported, I suppse). This is all just theory, but maybe it helps someone to get to the bottom of this.
Comment #144
eelkeblokI am happy to report that the patch for #3069220: Drupal 9 Deprecated Code Report does indeed solve my issue (there is more to that issue than the title, although it looks like the changes are in fact needed to get the module to be compatible with D9).
Comment #145
bbralaThe patch to jsonapi_extra's has been released, please update if you have these issues since that should filter out some noise.
Comment #146
podarok#124 patch for Drupal 8.8.3 does work. Thank you, BR0kEN
Comment #147
dilipsingh02 CreditAttribution: dilipsingh02 as a volunteer and at SynapseIndia Outsourcing Pvt. Ltd. commentedThanks,
I have tested this patch for core 8.9.3 and its working for me.
Comment #148
elizoller CreditAttribution: elizoller commented#124 Patch worked on 8.9.4
Comment #149
dilipsingh02 CreditAttribution: dilipsingh02 as a volunteer and at SynapseIndia Outsourcing Pvt. Ltd. commentedComment #150
bbrala@podarok, @dilipsingh02, @elizoller
I have a question for you guys; Do you use jsonapi_extras? If so, what version do you use? I still kinda suspect this was a jsonapi extra's issue, which was adressed recently. But to be sure we would need some confirmation.
Comment #151
damondt CreditAttribution: damondt commentedLatest patch #141 works for 8.8.8
Comment #152
dilipsingh02 CreditAttribution: dilipsingh02 as a volunteer and at SynapseIndia Outsourcing Pvt. Ltd. commentedHi @bbrala,
No I am not using "jsonapi_extras" on my website.
Thanks
Dilip
Comment #153
mglamanThis happens on install with a Drupal profile using JSON:API, regardless if using Extras or customizations, it seems.
Comment #154
Arlina CreditAttribution: Arlina at Chapter Three for Apigee commentedIn our case, we got this error trying to install commerce_product and other commerce submodules on a Drupal 8.9.1 site with core JSON API installed (not using jsonapi_extras). Patch #124 worked.
Comment #155
naveenvalechaI also encountered with this issue today with Core json API and Json API extras installed. I was installing commerce_product and its dependencies. I used the patch in #124 which fixed the issue. We're using the Drupal core 8.9.2.
Comment #156
podarok@bbrala - no jsonapi_extras in my case
Comment #157
Nicolas S. CreditAttribution: Nicolas S. commentedPatch # 124 helped me find my problem
Thks
Comment #158
bbralaWell, think it is safe to assume by now it i not jsonapi extra's that is creating this issue.
Comment #159
gabesulliceThis is clearly a major problem for many people, but I'm a bit lost by the current direction of the issue.
I felt that by comment #68, we had a pretty targeted patch that would help identify the source of the people's problems and that we had narrowed down the root cause to one of three non-JSON:API problems: bad configuration, a poorly defined custom base field, or JSON:API Extras.
In #71, I tried to summarize the state of the issue and provide a generally applicable recommendation that would help people debug the source of their broken config/code. #74 confirmed that this recommendation worked well. Getting the patch recommended in #71 to be committed seemed like the path of least resistance.
Around #84, this issue roared back to life, but it was never stated why the recommendations proposed in #71 were inadequate.
@BR0kEN, your series of patches starting in #89 are interesting and I like the direction that they're going, but by #141, they're modifying so much code that it's hard for me to spot the material changes.
I understand that many people find
array_(map|reduce)
difficult to follow in comparison to aforeach
loop, but changing those here makes this patch very difficult to review. I sincerely appreciate all the fixes you made to docblocks ase well, but again, I think they distract from the material changes that you're proposing.As I said, I do like direction you went because it makes JSON:API more internally consistent, but I wonder if your patch doesn't actually belong in #3105318: Add a public API for aliasing resource type names. What would you think about moving you changes there and finishing that patch?
If you would like to move all of the the docblock and array_reduce => foreach changes to a small, separate "Task" issue, I promise to review it and quickly RTBC it. Then this issue (or #3105318 if we move there) can be hyper-focused on material changes.
Unfortunately, I think this needs to be moved out of RTBC because it's not really ready to be committed without a proper review.
Comment #160
Hong Cai CreditAttribution: Hong Cai commented#124 works for me since drupal 9 has involved jsonapi into core, as long as the targeted module is drupal/core instead of drupal/jsonapi:
"drupal/core":{
"Argument 2 passed to jsonapi":"https://www.drupal.org/files/issues/2020-06-06/2996114-141.patch"
},
then it worked like a charm.
Comment #161
jwilson3Patch in #141 worked nicely on 9.0.7 to identify an error I missed after merging a feature branch that contained an entity reference field that was pointing to a node bundle type (events, plural) that had been renamed (event, singular) in the mainline branch.
Helpful warning message upon drush cr after applying this patch:
Thank you to all who've worked hard on this issue.
In addition to the NW changes requested in #159, my only humble suggestion would be to replace "Please take action" with something a little bit closer to the underlying problem, that might help someone in a Google search looking for better instructions.
Comment #162
BR0kEN@gabesullice I removed everything unrelated to the problem from #141 (I don't know how you guys read those constructions with
array_
functions without the 100% cognitive load of your CPU).I think that this issue should be prioritized over #3105318: Add a public API for aliasing resource type names because the new feature is new potential issues. It's a good idea to develop that feature with this improvement as it touches the same field. Otherwise, this patch may stop working, making all the work here ignored and require it to be started from scratch.
@jwilson3 any suggestions? Nothing comes to my mind.
Please take action
- is a universal phrase: either remove the field and entity type, or reconfigure the field, or remove the whole site - there is no unique action. Its presence means that something won't work correctly if you do not take action while the main helpfulness of that message is to point to the problematic field and entity type+bundle.Comment #163
mxr576I have custom, handwritten content entities in a project in custom modules. Some entity references taxonomy terms via
BaseFieldDefinition::create('entity_reference')
that only lives in the site configuration. By using #162 patch and installing the site with the built-in config installer I see these (false) warnings:[warning] The "XY entity" at "YZ entity:" references the "ZW term" entity type that does not exist. Please take action. ResourceTypeRepository.php:472
I am not sure how this could be resolved... Maybe not displaying these warnings if config sync is running?
Comment #164
gabesulliceThanks @BR0kEN! I'll review that soon!
@mxr576, I think that the patch @mglaman referenced should resolve your issue #3086307: Improve installer performance by ~20% by rebuilding the router after the entire installation is complete rather than after each module. Once Drupal 9.1 is released, you can remove the patch.
Comment #165
mxr576@gabesullice thanks for the suggestion, I tested the committed patch but it does not solve the issue completely that I reported in #163. Although the number of those warnings have decreased by ~80%. Maybe something still does a rebuild which causes this?
Comment #166
jsutta CreditAttribution: jsutta commentedPatch #124 worked for me on D8.9.8.
Comment #167
gabesulliceThanks @BR0kEN! I think this is super close. Review below. I've addressed all of my own concerns (all about wording and naming, nothing mechanical). I've also made a test only patch.
IMO, this will be good to go if tests and phpcs look good.
This is nitpicky, but I don't think we should have a test named "hack".
For the most part, I like this. I modified this a bit to be a little less coupled to all these static methods. Hopefully that will save a little time in the future.
Moved this to
RelatedResourceTypesTest
s/typeName/type name
It's uncommon for core to reference contrib modules. I think I can word this in an agnostic way.
Good comment. I moved this to the class docblock and made an argument why this test is good for core even if we ignore contrib.
Comment #169
BR0kENThe core of the fix remained unchanged which means I like this patch. There is a resolution of this issue blinking on the horizon 😃
Comment #170
bbralaThink i need to check how this patch works with extra's tomorrow see if there is some redundancy between those two. I did add some code of the patch to extra's a while back to fix some general issues.
Comment #171
bbralaI've went through the code, looking good. Good work everyone! RTBC ^^
Comment #172
gabesulliceWOOT 🥳 Thanks @bbrala!
Comment #173
gabesullice#3105318-41: Add a public API for aliasing resource type names is the follow up issue for the
@todo
s introduced in this patch.Comment #176
alexpottI've tried to credit everyone who has worked on this issue either by providing patches, reviews that affected the patch or with comments that have given the direction. It's hard because there are so many comments - which also go to show how important this is to land.
The one outstanding concern I have is #163/#165 - I don't think the answer in #164 is quite right - it this patch introduces warnings during the installer that weren't happening before then it'll introduce new problems because people will see a warning for something that is not correct.
I think the easiest way forward here is to recognise that the installer is a rare environment for a site to be it and be pragmatic.
Let's change the else to
elseif (!\Drupal\Core\Installer\InstallerKernel::installationAttempted()) {
here and add a comment saying only trigger the warning after a site is installed to avoid incorrect error messages.Once this has been addressed this issue can be put back to rtbc.
Comment #177
BR0kENComment #178
alexpott@BR0kEN thanks for the patch. Just a tiny tiny nit.
I'd do
else(!InstallerKernel::installAttempted())
Because there's no need to run this code in the happy path. And if this is the only call to it then we can avoid autoloading the code. Yep it is a micro-optimisation for sure but I'm not sure the local variable improves things.
Comment #179
jungleOne more nitpick: type hint should be added. Addressing this and #178, and let's land it to 9.2.x first.
Comment #180
jungleunskip is a new word which failed the spell checking, adding it to the dictionary.
Comment #182
jungleFixing
(Edited: wrong patch and interdiff file names, sorry, should be 2996114-182.patch and interdiff-180-182.txt)
Comment #184
jungleIt's a random fail.
Comment #185
bbralaPerhaps rephrasing is better. `Remove in issue https://www.drupal.org/project/drupal/issues/3105318`
Here :)
I think its wise to try and avoid new word if possible. Change that and ill RTBC again ^^
Comment #186
jungle#185 good point, thanks @bbrala!
Comment #187
bbralaThanks, well done. Looking good ^^
Comment #188
jungleBetter to have a meaningful method name probably, testResourceTypeRepository() or ?
Comment #189
bbralaAh yes, `testRepositoryResourceTypeNameAliassing` might be better. Coffee time o_O
Comment #190
jungleChanged as suggested in #189. I doubt that it is not being detected as I have not found the evidence here that it's being executed. Stay RTBC.
Comment #191
bbralaBetter, thanks :)
Comment #192
alexpottCommitted and pushed e82556c355 to 9.2.x and 090a1cf064 to 9.1.x. Thanks!
Backported to 9.1.x as this is only improved error reporting and a lot of people are running into this error.
Fixed spelling mistake on commit.
Comment #195
rpsuAny change patch (or some future modification of it) in #124 will get committed to 8.9.x?
Comment #198
simohell CreditAttribution: simohell commentedThe patch from #124 would not apply with 8.9.19 so I updated the patch and it gets applied.
Hope to get my site updated to D9 soon, though.
Comment #199
bsains CreditAttribution: bsains commentedThanks @simohell for the patch... beat me to it. However I think you've missed the field validation that was updated in the core for 8.9.19 as per the security advisories. We don't want to roll any of those changes back.
Specifically -
$field->isFieldEnabled()
See attached patch.
Comment #200
bsains CreditAttribution: bsains commentedLooks like I missed some of the tests from #124 - Rerolled.
Comment #201
bsains CreditAttribution: bsains commentedNot my morning... re-added with correct comment #
Comment #202
mxr576Please always attach interdiffs with previous patches. #124 was the last known good patch, please use that as a base.
@brains an interdiff with #198 would be also beneficial if you believe something was left out.
Comment #203
simohell CreditAttribution: simohell commentedPatch 201 seems legit. I think I did miss that one thing. I'll add the interdiffs.