Nokogiri error in engine

[codeclimate]

Nokogiri error in engine

[pbrisbin]

Hi @caiofbpa sorry about the trouble here.

undefined method `value' for nil:NilClass

This happens when csslint reports a node our XML parsing code doesn't expect. We attempt to grab attributes we expect to be there, then we ask for their #value and get this classic Ruby error when they weren't.

We've run into, and fixed, this particular bug a couple times. When and how it happens is a function of your underlying project code, which is why it may only happen on certain codebases.

Would you mind ensuring you're on the latest version of everything? If on OS X,

brew update && brew upgrade codeclimate

If the problem persists, the quickest way to figure it out would be to look at the raw XML report out of csslint and see if there are unusual or unexpected nodes we need to be handling (or ignoring). I'd be happy to do that debugging, if I'm able to access your project -- if it's not open source, maybe you'd feel comfortable emailing me a tarball? [email protected]

[caiofbpa]

Hello @pbrisbin, thanks for the response.

I updated codeclimate-cli and the error persists.

Unfortunately I can't provide you a tarball due to licensing issues.

What I can suggest (and maybe open a PR for) is better error handling in those cases, printing the unexpected XML in a meaningful manner instead of just going bananas without prior notice. I'll be taking a look at this during the week. Any help is appreciated!

[caiofbpa]

I inspected the raw XML and the engine's code, and there's something odd:

In csslint.rb you try to read the identifier attribute:

lint = node.attributes
check_name = lint["identifier"].value
check_details = CheckDetails.fetch(check_name)

There's no such attribute in the XML output (format is checkstyle-xml, same used by the engine):

<error line="40" column="1" severity="warning" message="Element (ul.footerlist) is overqualified, just use .footerlist without element name." source="net.csslint.Disallowoverqualifiedelements"/>

Maybe csslint unexpectedly changed the format, and we should check the error category from the source attribute instead of the identifier?

[caiofbpa]

Anyway, when I surrounded the code that parses the node and prints the issue in a begin/rescue block, the engine started to work fine on my codebase. I tried to capture the errors in the rescue block but nothing pops up. I'll open a PR with this code change and we can discuss different approaches from there, WDYT?

[pbrisbin]

identifier is a custom attribute we add in our fork of csslint, which the engine runs.

https://github.com/codeclimate/csslint/blob/00785e88c93174f601049a860fb38bfe344c1c4b/src/formatters/checkstyle-xml.js#L112

If you were trying to inspect the raw XML the engine sees, you'd have to install our fork: https://github.com/codeclimate/codeclimate-csslint/blob/master/Dockerfile#L11-L12

[caiofbpa]

So as it turns out, the error was:

key not found: "identifier"

And it occurred on the following XML:

<error line="1" column="1" severity="error" message="Unexpected token '"/>

Which occurred when analyzing a minified and gzipped file in my codebase (yikes!) I'll make sure to add this file to my exclude_paths.

Anyway I guess the engine could deal with such error in a better way, so #26 is open for us to discuss the best approach to take when this stuff happens.

[caiofbpa]

I guess you can also close this issue, because now people will have a better clue about what went wrong when the engine fails. 😄

[pbrisbin]

It's definitely still a bug to be fixed, but I think it's in https://github.com/codeclimate/csslint. It seems that code outputs an error node when it encounters a parser error, but we've not changed it to include an identifier in that case.

I'll leave this issue open here until that's addressed.

[pbrisbin]

@caiofbpa would it be possible to send me just the specific file that is causing your error? Or a file that contains similar problematic content? I've been trying to reproduce this error and not having much luck so far.

[caiofbpa]

I emailed you the file, GitHub says it's not supported here.

[caiofbpa]

It should be easy to reproduce though: get any .css file, minify it and gzip it. It should be enough.

[pbrisbin]

Thanks very much, I'll look at this closer this week and report back.

One thought: the fact that it's even looking at a *.gz file is probably undesired. I thought we had made changes to ensure it only looks at *.css.

[pbrisbin]

Oh, I see now. The file's named .gz.css -- that explains that.

[dblandin]

Hi @caiofbpa, are you still experiencing this issue?

@pbrisbin Did anything come out of your investigation?

[pbrisbin]

@dblandin Sorry for dropping the ball here. Here's the result of investigation so far:

Since the file is named .gz.css (instead of .css.gz), csslint is (for some definition) right to try and process it. In other words, there's no bug (that I see) in choosing what files to process. This is just an edge case that will happen and (I believe) isn't worth addressing directly.

When csslint encounters such a case (gzipped content in a .css file), the XML produced does not include our custom identifier key. For the life of me, I can not find the spot in csslint's source where this XML is generated. All XML generation I see has our custom identifier addition. This is the remaining bug (I think) and might be worth opening as an Issue on the csslint repo and closing this one.

[pbrisbin]

I went ahead and created an Issue there, so I'll close this one.

[caiofbpa]

Just to let you know, I don't have this issue anymore, because I manually ignored the .gz.css file. Other people who may run into the same issue will see what file caused the engine to fail due to #26. They can then check the problematic file, and manually ignore them like I did.