Log an error message when library definitions have a license with no URL defined

Here it is with ?? 'no URL'. Can't really think of a better message or what else to do here, short of validating the library definitions and enforcing a URL, which would mean triggering a deprecation message for this case I guess.

We should add a test that covers this case as well.

I'm happy to define that license definitions have a url and that colorbox should be fixed and this is just a fallback/defensive programming.

I think if we say that, we shouldn't silently fix this, and instead log an error message with the library name in it.

did the same for photoswipe: #3371315: Photoswipe library definition license is missing URL

Just got bit by this on latest 10.1 deploy.

Re-titling and updating the proposed resolution.

I encountered this problem in 10.1.1, it was caused by the lack of url in the views_slideshow module. I found no license file in this module. In order to avoid the same problem with other modules, I added this patch to the core

@Randy Tang that's similar to #3, but please see the discussion an issue summary - when a license is specified there should also be a URL, so instead we should log a more useful error. I think an assert() might be a better option here, then it would still be silent on production but caught a lot more quickly locally.

Hi,
This same issue is there for core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php file L160

Warning: Undefined array key "url" in Drupal\Core\Asset\CssCollectionOptimizerLazy->optimizeGroup() (line 160 of core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php).
Drupal\Core\Asset\CssCollectionOptimizerLazy->optimizeGroup(Array) (Line: 183)
Drupal\system\Controller\AssetControllerBase->deliver(Object, 'css_xZ1grJh6fKKy0PyENOxVLpswGrM9ycqavFyz8YvMrmw.css')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 583)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 166)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Same here. Getting the error from both CssCollectionOptimizerLazy and JsCollectionOptimizerLazy and the only module I can find that's missing a license URL is owlcarousel, which has both js and css in its library.

@scotwith1t yes it's missing the license URL (https://git.drupalcode.org/project/owl-carousel/-/blob/8.x-2.x/owlcarous... for reference). Can you open an issue against owlcarousel? This issue won't get rid of the error, it will just make it more obvious which module is the culprit.

Warning: Undefined array key "url" in Drupal\Core\Asset\JsCollectionOptimizerLazy->optimizeGroup() (line 174 of core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php).

Same issue here, it is intermittent throughout the site including in the admin panel whilst moving around that.

Drupal 10.1.2
Php 8.1

Manually installed the patch as could not get it to work with the composer, will see how it goes over the next 24-48 hours

Got this back again - when I upgraded to 10.1.3, so reapplied the patch.

Can this be added to the new update to avoid having to add it every time a new patch comes out?

Question about this, we have some custom modules which add libraries that have a proprietary license only for the project and have no URL at all. Should I just provide an empty string? I want to avoid running into PHP notices.

@Dave yes but it'll show up in aggregates, so make sure that's what you want.

#2258313: Add license information to aggregated assets

I had the same problem but with CSS
Warning: Undefined array key "url" in Drupal\Core\Asset\CssCollectionOptimizerLazy->optimizeGroup() (line 160 of /app/docroot/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php)

I made this css_optimizer_url.patch for css and now this warning is gone.
I tested this with drupal 10.1.4

Update the patch above and fix the error for js optimization.

We're getting this message when using the superfish module 8.x-1.7. In that module, the libraries info is provided by functions in the superfish.module file. The license info is provided within those functions, but that info does not seem to be passed to the functions in the JsCollectionOptimizerLazy.php file.

I also agree that the errors do not provide helpful info to identify which library is missing the URL.

Regarding tests from #4 #3359497-4: Log an error message when library definitions have a license with no URL defined we have a library that has this in core:
public/core/tests/Drupal/Tests/Core/Asset/library_test_files/licenses.libraries.yml:63
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/...

And this tests it should be returning without a URL.
\Drupal\Tests\Core\Asset\LibraryDiscoveryParserTest::testLibraryWithLicenses
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/...

I guess we'd just need to try to render the result of that parser to get it to fail?

To clarify for followers: patches in #3 and #13 just target js libraries, #22 css only and #23 targets both. All use the same work-around.

But note: This is a work-around only (not a fix). Follow the discussion to understand why we need to track missing urls for library licenses and that we just need another way to log it (without frontpage warnings). As long as you get warnings and notices on frontpages and you really need to surpress them, this patches are useful (also in production). But ONLY if you are sure that you do NOT need the license url in your case.

In my case it throws:
Warning: Undefined array key "url" in Drupal\Core\Asset\JsCollectionOptimizerLazy->optimizeGroup() (line 174 of /var/www/html/web/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php).

This is the undefined "url" key. To avoid this situation I check with isset().

There's far too many errors in contrib that result in annoying warnings.

I chased down fontawesome, created a patch for it, but when I log into my site, I notice other warnings from other modules. Far too many to keep track of.

So I'm going to use patch#23.

Patch #23 uses the PHP 8.1 ?? syntax,
this is appropriate and good for Drupal 10+

I went ahead and applied the last patch that [email protected] posted (#30) with success. I think we need to find an overall solution to this compatibility issue, as this is a very uncomfortable error that isn't site breaking. How about we add a warning status message and/or a warning log message instead of a php warning error? This will make the warning comfortably yellow while also notifying developers that a change needs to be made. It might also be best to limit this error to certain pages (i.e. status report and available updates). If I have time, I will post a patch including the above changes. I hope we get this resolved, as it is very distracting while developing.

I agree with @joseph.olstad in #28, noting that while #23 may not be the best approach overall, it is the most effective for quelling the PHP log messages and the one we are currently using in production. Thank you, @sakonn.

The patches in #27 and #30 do not seem to introduce new solutions or expand the scope of this issue, which I would prefer to avoid.

I will write a test and move this to a merge request while hiding all patches except #23. Additionally, I will take @gloryrose6’s sentiment in #31 into account and consider adding a warning status to help direct the problem to the appropriate space and context. The solution in #23 effectively hides the problem and reduces PHP log messages, which currently misrepresent the issue as a code problem when it is actually a policy matter and should be acted on as such.

Error logs on a few of my sites were getting filled with warnings and I could not find culprit in backtrace. Applying this patch to core (#23) has resulted in clean logs, finally!

I took the patch from #30 and just added a isset for the license

Just checking in on the status of this issue.

Would it make sense to merge the patch with isset() with core to suppress the warning on the front page and then create an issue to work on providing useful logging of licenses without URL's.

For now, in order to remove the notice on production sites, my workaround is to disable the "Help" block to suppress the warning. It's not optimal because it removes all notices but for now it'll have to do.

Cheers,
Andrew

New MR with reroll

Thanks but issues need to land in main first. Also appears to have been tagged for test coverage that’s still needed

We've been using this patch for quite a while now in many builds.