Contrib.social

Error handling for AssetDumper in CSS/JsCollectionOptimizer

Open on Drupal.org →
Created on 20 January 2017, over 8 years ago
Updated 14 February 2023, about 2 years ago

Problem/Motivation

Currently the aggregator file generated in below steps:
1. HtmlResponseAttachmentsProcessor::processAssetLibraries() calls AssetResolver::getCssAssets()
2. AssetResolver::getCssAssets() loads from cache_data if already generated css/js group available
3. If not available calls CssCollectionOptimizer::optimize()
4. here, assets get grouped and generated hash based on lib in that group
5. Call to AssetDumper::dump() made which returns uri of generated file for the group.
6. Hash generated in step 4 and uri from step 5 are saved in state (drupal_css_cache_files)

There is a possibility of getting FALSE for uri in step 5. However it's not handled in CssCollectionOptimizer::optimize(). It gets saved as empty in state, but when we reach same method next time, try generate the uri again if empty. However we never visit this part (with out cache clear) as this data also getting cached in step 2.

Proposed resolution

Handle empty $uri from AssetDumper::dump() on both CssCollectionOptimizer::optimize() and JsCollectionOptimizer::optimize(). It can be one of those below:
a) throw an error
b) Try regenerate the URI
c) do not cache step 2 if one of the group has empty $uri.

Remaining tasks

- Discuss
- Patch

🐛 Bug report
Status

Closed: outdated

Version

10.0 ✨

Component
Asset library  →

Last updated 3 days ago

No maintainer
Created by

🇬🇧United Kingdom vijaycs85 London, UK

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 2 years ago →
    🇦🇺Australia mstrelan

    This was a daily triage target for the Bug Smash Initiative. As per #2 we need a test for this. We also probably need to restore the error suppression and potentially raise a new issue for that as per #12 and #16. I think we also need an IS update with a concise overview of the problem before jumping in to the details of how it happens, if I understand correctly that would be something like "empty URLs can be cached and assets end up point to /" until the next cache clear".

  • Status changed to Needs review over 2 years ago
  • Comment over 2 years ago →
    🇮🇳India Abhisheksingh27

    Adding reroll for D10
    please review

  • Comment over 2 years ago →
    🇬🇧United Kingdom catch

    This should be a non-issue in 10.1.x after 🐛 Stampedes and cold cache performance issues with css/js aggregation Fixed - the URLs are generated separately from the file being written to disk or not.

    Question though whether we still want to try to fix this in 10.0.x/9.5.x

  • Status changed to Closed: outdated about 2 years ago
  • Comment about 2 years ago →
    🇦🇺Australia mstrelan

    The patch in #21 does not apply to 10.0.x and does not address the points in #20. That said, I don't think this is worth fixing now. The issue has been open for 6 years with minimal interest. Within a year 9.5.x will be EOL and 10.1.0 will be released. Closing outdated, feel free to re-open if you have strong opinions about fixing this.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024