October: Combiner assets are not generated on nginx server

[meinmotion]

When I deploy my code to a nginx server the combined assets are not generated after upgrading my OctoberCMS to version 459. It looks like the new way of writing the combined assets including the extension is causing this issue. When I rollback to version 458 everything works again. On my development machine I'm using Apache and that works fine. Maybe I'm overlooking something in the settings? Or in the nginx config?

[LukeTowers]

@meinmotion what exactly do you mean by not working?

Also what is your NGinx config? It's probably set to try to handle all asset extensions (.js, .css) itself and (crucially) not respond to those requests not being found by passing them to October.

[meinmotion]

Hello luke,Thanks for your quick response. The url's like: combine/c76f81894d6bd9ae814e5326cae41aaf-1569956958.css are giving an 404 error.

my nginx config is the standard one found in the October documentation:

location / {
    # Let OctoberCMS handle everything by default.
    # The path not resolved by OctoberCMS router will return OctoberCMS's 404 page.
    # Everything that does not match with the whitelist below will fall into this.
    rewrite ^/.*$ /index.php last;
}

# Pass the PHP scripts to FastCGI server
location ~ ^/index.php {
    # Write your FPM configuration here

}

# Whitelist
## Let October handle if static file not exists
location ~ ^/favicon\.ico { try_files $uri /index.php; }
location ~ ^/sitemap\.xml { try_files $uri /index.php; }
location ~ ^/robots\.txt { try_files $uri /index.php; }
location ~ ^/humans\.txt { try_files $uri /index.php; }

## Let nginx return 404 if static file not exists
location ~ ^/storage/app/uploads/public { try_files $uri 404; }
location ~ ^/storage/app/media { try_files $uri 404; }
location ~ ^/storage/temp/public { try_files $uri 404; }

location ~ ^/modules/.*/assets { try_files $uri 404; }
location ~ ^/modules/.*/resources { try_files $uri 404; }
location ~ ^/modules/.*/behaviors/.*/assets { try_files $uri 404; }
location ~ ^/modules/.*/behaviors/.*/resources { try_files $uri 404; }
location ~ ^/modules/.*/widgets/.*/assets { try_files $uri 404; }
location ~ ^/modules/.*/widgets/.*/resources { try_files $uri 404; }
location ~ ^/modules/.*/formwidgets/.*/assets { try_files $uri 404; }
location ~ ^/modules/.*/formwidgets/.*/resources { try_files $uri 404; }
location ~ ^/modules/.*/reportwidgets/.*/assets { try_files $uri 404; }
location ~ ^/modules/.*/reportwidgets/.*/resources { try_files $uri 404; }

location ~ ^/plugins/.*/.*/assets { try_files $uri 404; }
location ~ ^/plugins/.*/.*/resources { try_files $uri 404; }
location ~ ^/plugins/.*/.*/behaviors/.*/assets { try_files $uri 404; }
location ~ ^/plugins/.*/.*/behaviors/.*/resources { try_files $uri 404; }
location ~ ^/plugins/.*/.*/reportwidgets/.*/assets { try_files $uri 404; }
location ~ ^/plugins/.*/.*/reportwidgets/.*/resources { try_files $uri 404; }
location ~ ^/plugins/.*/.*/formwidgets/.*/assets { try_files $uri 404; }
location ~ ^/plugins/.*/.*/formwidgets/.*/resources { try_files $uri 404; }
location ~ ^/plugins/.*/.*/widgets/.*/assets { try_files $uri 404; }
location ~ ^/plugins/.*/.*/widgets/.*/resources { try_files $uri 404; }

location ~ ^/themes/.*/assets { try_files $uri 404; }
location ~ ^/themes/.*/resources { try_files $uri 404; }

[meinmotion]

I see that my standard nginx config also contains:

location ~* \.(?:css|js|jpg|jpeg|gif|ico|png|bmp|pict|csv|doc|pdf|pls|ppt|tif|tiff|eps|ejs|swf|midi|mid|ttf|eot|woff|otf|svg|svgz|webp|docx|xlsx|xls|pptx|ps|class|jar|woff2|less|scss)$ { 
    add_header Pragma public;
    add_header Cache-Control "public, must-revalidate, proxy-revalidate";
    add_header Access-Control-Allow-Origin *;
    access_log off;
    expires 30d;
}

[LukeTowers]

Hmm, I'm not really an NGinx config expert, @alxy @w20k @petehalverson are any of you good at it?

[meinmotion]

I removed the css|js from the location and now it works. It's strange that changing the headers have such an impact on this. Thanks for you response.

[LukeTowers]

@meinmotion Ah, I see why that happened. location ~* \.(?:css|js)$ { is more specific than location / { so it gets run instead, which means that the request never gets passed off to October. If you changed it so that any /combine URLs are told to be handled by October in addition to having the header processing then you could have both working.

[slowpokefarm]

So, is there a working solution to this? Documentation still does not contain anything about this case

[w20k]

@slowpokefarm most probably you've issues with your Nginx config. Paste it here we could look it over.

[slowpokefarm]

@w20k I have additional rules for for css/js, it worked well before that build, but now it fails just as OP has described.

server {
  server_name app.local;
  root        /var/www/app;
  index       index.php;

  gzip on;
  gzip_types text/plain text/css application/json application/javascript application/x-javascript application/octet-stream text/xml image/svg+xml image/png application/xml applica$

  client_max_body_size 100M;
  fastcgi_read_timeout 1800;

  location / {
    try_files $uri $uri/ /index.php?$query_string;
  }

  location ~ \.html$ {
    try_files $uri $uri/ /index.php?$query_string;
  }

  location ~ \.php$ {
    try_files $uri /index.php =404;
    fastcgi_split_path_info ^(.+\.php)(/.+)$;
    fastcgi_pass unix:/run/php/php7.2-fpm.sock;
    fastcgi_index index.php;
    fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
    include fastcgi_params;
  }

  rewrite ^themes/.*/(layouts|pages|partials)/.*.htm /index.php break;
  rewrite ^bootstrap/.* /index.php break;
  rewrite ^config/.* /index.php break;
  rewrite ^vendor/.* /index.php break;
  rewrite ^storage/cms/.* /index.php break;
  rewrite ^storage/logs/.* /index.php break;
  rewrite ^storage/framework/.* /index.php break;
  rewrite ^storage/temp/protected/.* /index.php break;
  rewrite ^storage/app/uploads/protected/.* /index.php break;

  listen 80; # managed by Certbot

  # ELB stores the protocol used between the client
  # and the load balancer in the X-Forwarded-Proto request header.
  # Check for 'https' and redirect if not
  # if ($http_x_forwarded_proto != 'https') {
  #     return 301 https://$host$request_uri;
  # }

  # Expire rules for static content

  # No default expire rule. This config mirrors that of apache as outlined in the
  # html5-boilerplate .htaccess file. However, nginx applies rules by location,
  # the apache rules are defined by type. A consequence of this difference is that
  # if you use no file extension in the url and serve html, with apache you get an
  # expire time of 0s, with nginx you'd get an expire header of one month in the
  # future (if the default expire rule is 1 month). Therefore, do not use a
  # default expire rule with nginx unless your site is completely static

  # cache.appcache, your document html and data
  location ~* \.(?:manifest|appcache|html?|xml|json)$ {
    add_header Cache-Control "max-age=0";
  }

  # Feed
  location ~* \.(?:rss|atom)$ {
    add_header Cache-Control "max-age=3600";
  }

  # Media: images, icons, video, audio, HTC
  location ~* \.(?:jpg|jpeg|gif|png|ico|cur|gz|svg|mp4|ogg|ogv|webm|htc)$ {
    access_log off;
    add_header Cache-Control "max-age=2592000";
  }

  # Media: svgz files are already compressed.
  location ~* \.svgz$ {
    access_log off;
    gzip off;
    add_header Cache-Control "max-age=2592000";
  }

  # CSS and Javascript
  location ~* \.(?:css|js)$ {
    add_header Cache-Control "max-age=31536000";
    access_log off;
  }

  # WebFonts
  # If you are NOT using cross-domain-fonts.conf, uncomment the following directive
  location ~* \.(?:ttf|ttc|otf|eot|woff|woff2)$ {
   add_header Cache-Control "max-age=2592000";
   access_log off;
  }
}

[w20k]

@slowpokefarm @meinmotion I believe it's because of this PR: https://github.com/octobercms/october/pull/4567

Now extension is included in your URL now. And most probably you'd have to add ignore statement for css|js rule if it contains your combiner URL. Should work in both cases.

If you need help in writing one - ping me!

[slowpokefarm]

@w20k I would really appreciate your help. I want to make combined assets working again and still have control over static assets headers. Which would be the best way to do it?
For now I removed the css|js block. I also tried to add try_files there, but I suspect it can break things in some cases.

[w20k]

@slowpokefarm @meinmotion
If I'm not wrong, that would be the quickest solution to fix this issue (an empty location for combined and extras. naming could be different). If I'd have a minute to write a proper Regex, will ping you back ;)
This an example for extras & combined, probably front-end combined have a different naming. Don't have any live projects wiht combiner to test those out :)

location ~* (extras|combined)\.(?:css|js)$ {
    #empty
}

 # CSS and Javascript
  location ~* \.(?:css|js)$ {
    add_header Cache-Control "max-age=31536000";
    access_log off;
  }

[LukeTowers]

We'll need to address this in some way for build 460 since it seems to be a breaking change for users with nginx configurations that hijack paths ending in .css and .js. We could either make it a configuration to use the extensions or we could improve our nginx documentation to ensure that people are having all nginx 404s handled by October as they should be doing already. @daftspunk @bennothommo @w20k thoughts?

[bennothommo]

@LukeTowers tbh, I'd be more inclined to roll back #4567 and approach it differently - ie. an actual CSS / JS file is generated that is web accessible. It shouldn't need to go through October.

[w20k]

@LukeTowers it's not in our setup, yet. But quite a common optimization for Nginx. We could just add a note in a setup guide.
Or a rollback as @bennothommo suggested ;)

[LukeTowers]

@bennothommo my problems with having an actual file generated is dealing with the old versions of the files, the different URL to the file we'd have to have, and how to handle if that file somehow goes missing (say if you're running multiple servers for a single October instance)

[Samuell1]

@LukeTowers combiner should work in way like webpack does, it generates manifest file with pointing original name of file to file name hash and it will change only if file content will change.

[daftspunk]

+1 to roll this back. It was attempted in the past and created more problems than it solved, also couldn't find a clear use case for it to be needed

[LukeTowers]

@daftspunk the use case is to help CDNs recognize the file types and cache them appropriately. @alekseyp can you chime in on this?

[daftspunk]

Needed and wanted are different. For example, if you want to include the suffix, the combiner should already support it without any change to the core logic.

<link href="{{ [
    'assets/less/vendor.less',
    'assets/less/theme.less'
]|theme }}.css" rel="stylesheet">

Even add some cache-busting if you like:

<link href="{{ [
    'assets/less/vendor.less',
    'assets/less/theme.less'
]|theme }}.css?1234" rel="stylesheet">

[slowpokefarm]

I don't think generating an actual file is a good idea. For example, we have multiple instances of the app with redis cache connected to all of them, which allows instances to serve the same cached combined assets. It works well the way it is. Something that I would appreciate though is a mention in documentation about how to handle extension specific locations in nginx configuration to keep it working with newer builds.

@w20k your solution sadly didn't work, maybe I didn't understand the approach correctly.

What I did for now is I added try_files directive after adding asset headers in (css|js) location, so it looks like this now:

  # CSS and Javascript
  location ~* \.(?:css|js)$ {
    add_header Cache-Control "max-age=31536000";
    access_log off;
    try_files $uri $uri/ /index.php?$query_string;
  }

Should be no issues and 404 are handled within framework instead of nginx if I understand the concept right.

[Samuell1]

@slowpokefarm Even when you have multiple instances its still different name of combiner file.

[slowpokefarm]

@Samuell1 is it? Even if so there might be an issue where one instance generates the name, then the request for asset is routed to a different instance that does not have this file.

[Samuell1]

@slowpokefarm yeah when i used combiner it created per instance different combiner hash

[LukeTowers]

Excellent point @daftspunk, I'm going to revert #4567 and add a note to the docs about adding the asset extensions

[LukeTowers]

Fixed by https://github.com/octobercms/october/commit/7e98e199a46040dbf22b8211d33115ffae780e32