Appends the load paths to error message #313
Conversation
When `resolve!` raises an error, it would not display the paths that were used to look up the assets. If we raise this error we can be even more helpful and display the `config[:path]` as well. This makes it easier for users to debug their asset paths. References rails#252
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @schneems (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
|
|
I like the direction, i'm thinking that it's better to list out the paths like an ordered list. It might be a much longer error, but it will be more readable The problem here is that we accidentally put out SO much information it makes debugging harder. This got me thinking that maybe we could either add some kind of command that can be used for debugging like
Another alternative could be to raise a special Sprockets exception and have the sprockets middleware catch and display a debugging page in development like Rails does with Let's move forwards with the paths in the config on different lines You can test the exception in test "adds paths to exceptions" do
random_path = SecureRandom.hex
@env.append_path(random_path)
error = assert_raises(Sprockets::FileNotFound) do
uri, _ = @env.resolve!("thisfiledoesnotexistandshouldraiseerrors", {})
uri
end
assert_match /#{ random_path }/, error.message
endHow does that sound? |
|
That does sound like a good idea.
Is that something that you'd find necessary? |
That's tough, there's not always a basename given. For now let's put more debug info in. We can work on refining it later. |
|
Alright @schneems it looks like this now: Thank you for your input. |
|
What is the problem with AppVeyor?
I don't understand this message. |
|
3.x doesn't work on windows it's fine. Merging in. Then going to forward port to master. |
When
resolve!raises an error, it would not display the paths thatwere used to look up the assets. If we raise this error we can be even
more helpful and display the
config[:path]as well. This makes iteasier for users to debug their asset paths.
I am unsure whether you like the way I introduced the load paths into the error message. Please see below.
Original error messages:
New error messages:
I could not get the test to pass, because of the double escaping quotes:
Any help would be appreciated.
References #252