Opened 3 years ago

Closed 2 years ago

Last modified 17 months ago

#18986 closed Cleanup/optimization (fixed)

Improve error message for broken CSS in CachedStaticFilesStorage

Reported by: aaugustin Owned by: aaugustin
Component: contrib.staticfiles Version: 1.4
Severity: Normal Keywords:
Cc: zyegfryed Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

When a CSS file references a non-existing image, CachedStaticFilesStorage raises an exception — which is the right thing to do at this point (see #18958).

But the backtrace doesn't tell which file contains the error. As a consequence, when switching a large project to CachedStaticFilesStorage, these errors are difficult to locate and fix.

To improve this situation, we could change the output to show the name of each file before processing it:

Processing file xxx… DONE
Processing file xxx… FAIL

The goal is to make available all the information that could be useful for debugging, not to change the implementation of CachedStaticFilesStorage.

Change History (11)

comment:1 Changed 3 years ago by aaugustin

  • Component changed from Uncategorized to contrib.staticfiles

comment:2 Changed 3 years ago by aaugustin

See also #19650.

comment:3 Changed 3 years ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned

comment:4 Changed 3 years ago by aaugustin

comment:5 Changed 3 years ago by aaugustin

  • Has patch set
  • Patch needs improvement set

The current pull request is on the right track, but I'd like to avoid catching an exception and writing to stderr inside CachedFilesMixin.post_process. The error handling and display is the responsibility of the management command itself.

Unfortunately the current API makes this non-trivial. collectstatic contains the following code:

            processor = self.storage.post_process(found_files, dry_run=self.dry_run)
            for original_path, processed_path, processed in processor:
                # display the file and whether it was processed

processor is a generator. If an exception is raised while processing a file, the generator exits and we cannot recover the file path.

One solution would be to extend the values of processed to accept True, False, or an exception. post_process could catch exceptions and pass them to collectstatic; collectstatic would display an helpful message then re-raise exceptions. I find it slightly annoying to introduce a "boolean or something else" value but I don't have a better idea at this point.

comment:6 Changed 2 years ago by zyegfryed

I'm sharing your point of view.
Another solution would be to yield an exception and test the type of the generated value (see http://forums.fedoraforum.org/showpost.php?p=1132791&postcount=4). What do you think?

comment:7 Changed 2 years ago by zyegfryed

  • Cc zyegfryed added

comment:8 Changed 2 years ago by zyegfryed

I've updated the patch according to your concern : delegating error handling to the management command.
Let me know whether you've thought of some other improvements.

comment:9 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 90fe9141ded9f7005546e9a66f31fd196f60e7e4:

Fixed #18986 -- Improved error message for missing files

in CachedStaticFilesStorage. Thanks zyegfryed for his work on the patch.

comment:10 Changed 17 months ago by anonymous

Or just scroll up to see the last file that was processed and you have the file name that caused the error!!! This is the largest waste of time on a patch i have ever seen! Just give us a warning that the file was missing and move on. Nobody cares!

Attempting to use CachedStaticFilesStorage with django 1.6 fails on "Post-processing 'admin/css/base.css' failed!"
ValueError: The file 'img/nav-bg.gif' could not be found with <require.storage.OptimizedCachedStaticFilesStorage object at 0x02D8F790>.

Yes that's right even django's very own admin breaks CachedStaticFilesStorage. Maybe you should start by patching that?

comment:11 Changed 17 months ago by aaugustin

This sounds like a problem with your installation. The file exists in Django 1.6: https://github.com/django/django/blob/stable/1.6.x/django/contrib/admin/static/admin/img/nav-bg.gif

Given the level of useless aggressivity you're demonstrating, I'm sorry, but I'm not going to help you and I wish you were using another web framework. Thanks for your understanding.

Last edited 17 months ago by aaugustin (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top