"Only file JavaScript/CSS assets can be optimized" errors in logs

Opened MR
For context, I suspect we may have an infrastructure issue somewhere, possibly the reverse proxy serving stale pages.
Still, if that is the case, drupal should not fill the logs with errors. The changes here will log BadRequestHttpException warnings just like other invalid asset requests.

Tests and steps to reproduce still todo.
But any feedback is welcome.

Static patch for composer.

@herved Thanks so much for jumping on this — I’m seeing the same thing in my logs, so really appreciate you taking this on!

RE The mention in the proposed solution

This would also cover 🐛 Assets paths in CSS no longer rewritten when aggregation is enabled Active which is a consequence of the change in #3414173.

I wonder if it might be helpful to keep the scope of that one in its own issue, just to keep things a bit more focused? I realize they are related (because this JS one is where I copied from in 🐛 Website error Exception: "Only file CSS assets can be optimized" Active ) but painting them with the same brush is what brought us here in the first place.

Hi @joelpittet, I know I combined 2 issues into 1 here, which may not be ideal, but they are tighly coupled.
In my understanding, considering this issue here, and the changes I propose, external files would and should never enter the ::optimizeGroup method. So does it really make sense to check for minified === TRUE && type === external in ::optimizeGroup as you proposed there? Meaning the condition is therefore not needed.

Thanks, @herved — you might be right here, especially since you probably have a better handle on the internals of ::optimizeGroup() than I do.

If you haven’t already, feel free to take a look at the test in 🐛 Assets paths in CSS no longer rewritten when aggregation is enabled Active — it might be helpful to reuse or adapt parts of it to confirm whether ::optimizeGroup() is actually called in these cases. Even just running the test on its own could help show the problem is fixed. That kind of check might make the expected behavior a bit clearer.

I’m still a bit cautious about collapsing the two issues, since that overlap is partly what got us into this situation. But if you feel strongly that they belong together, I won’t block it. I do think the targeted fix in 🐛 Assets paths in CSS no longer rewritten when aggregation is enabled Active should resolve the CSS side of things.

Really appreciate the work you’re doing here — you might end up solving a broader problem in the process! I’m just laser-focused on cleaning up the regression I introduced, and if I expand the scope too much, I’m afraid I’ll cause another one 😬