Code

Opened 19 months ago

Closed 12 months ago

Last modified 6 weeks 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.

Attachments (0)

Change History (11)

comment:1 Changed 16 months ago by aaugustin

  • Component changed from Uncategorized to contrib.staticfiles

comment:2 Changed 15 months ago by aaugustin

See also #19650.

comment:3 Changed 15 months ago by aaugustin

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

comment:4 Changed 15 months ago by aaugustin

comment:5 Changed 15 months 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 15 months 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 15 months ago by zyegfryed

  • Cc zyegfryed added

comment:8 Changed 15 months 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 12 months 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 6 weeks 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 6 weeks 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 6 weeks ago by aaugustin (previous) (diff)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.