AMP prevents css assets from generating when aggregation is enabled on Drupal 10.1

I have tried this patch on one of my projects and it works. I am leaving this for others review and to be marked as RTBC.

+++ b/src/Asset/AmpCssCollectionRenderer.php
@@ -164,10 +165,14 @@ class AmpCssCollectionRenderer extends CssCollectionRenderer {
+            \Drupal::messenger()->addWarning('Failed to load the css file');

I feel we need to come up with better wording for this, and maybe use a watchdog message instead of a message to the user?

In any case, the patch in #2 works for us when aggregation is enabled. I'm going to leave the ticket in "Needs review" so the maintainers can chime in on the above mentioned issue.

Thanks!

The same issue on D10.1.6, patch #2 solved the issue. Thank you.

If anyone is still having issues after applying this patch and your environment has htauth on try turning it off.

I've created a MR that uses the provided patch at #2 🐛 AMP prevents css assets from generating when aggregation is enabled on Drupal 10.1 Needs review with the following changes:

1. Replacing Drupal static calls with dependency injection
2. Removing the $needs_rewrite check, so we always send the assets to the doRewrite() method. I've checked and without that, the custom fonts won't be loaded (404) since we were trying to get them using the relative one level up../. Screenshot attached trying to explain this error.
3. Removing the message inside the catch, replacing it with a proper logger if something goes wrong in the curl guzzle request.
4. Also added a to-do comment to the guzzle request since this was failing on my local env (using docker localhost)

I will also provide, in a separate branch another "solution" for this css not loading issue. Being more like a workaround to be able to keep using the file_get_contents approach, simply ignoring aggregation for css (exactly as this module is doing with the javascript) until we can find a working solution that applies to all environments (please not the curl guzzle request will also fail if sites are behind a proxy).

ok, so... I've tested https://git.drupalcode.org/project/amp/-/merge_requests/17 in one of our sites dev environment that requires to have a cookie setup in order to load the pages and the guzzle approach is failing with a 403 since that request does not have the cookie set. I imagine a similar thing would happen on sites behind a proxy and we know is also happening on a local docker environment.

I'll try to modify the MR re-writing the render method. Setting back to needs work.

I can also confirm that patch fixes the problem.

I agree with #4 about wording and instead of showing this error to the user it would be nice to add it to watchdog.

The MR looks like it's doing too many things. In our infrastructure we have varnish in front and the patch worked fine. Also with local docker environments, we use DDEV.

I don't understand the use case of a required cookie to load css assets, but I leave it for maintainers to double check it.

Thanks for all the contributions!