Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#18986 closed Cleanup/optimization (fixed)

Improve error message for broken CSS in CachedStaticFilesStorage

Reported by: Aymeric Augustin Owned by: Aymeric Augustin
Component: contrib.staticfiles Version: 1.4
Severity: Normal Keywords:
Cc: Sébastien Fievet 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 by Aymeric Augustin, 12 years ago

Component: Uncategorizedcontrib.staticfiles

comment:2 by Aymeric Augustin, 12 years ago

See also #19650.

comment:3 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin
Status: newassigned

comment:4 by Aymeric Augustin, 12 years ago

comment:5 by Aymeric Augustin, 12 years ago

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 by Sébastien Fievet, 12 years ago

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 by Sébastien Fievet, 12 years ago

Cc: Sébastien Fievet added

comment:8 by Sébastien Fievet, 12 years ago

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 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 90fe9141ded9f7005546e9a66f31fd196f60e7e4:

Fixed #18986 -- Improved error message for missing files

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

comment:10 by anonymous, 11 years ago

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 by Aymeric Augustin, 11 years ago

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 11 years ago by Aymeric Augustin (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top