Node-sass: segmentation fault when parsing invalid sass

[chrisspiegl]

This problem was already discussed in issue #7. But as far as I tested it, it is still a problem.
I am using node-sass via gulp-sass. gulp-sass use the current version of node-sass (^0.9).

If I try to compile a scss file with an missing ; it writes segmentation fault to the console and crashes.

[am11]

I fixed a similar issue with https://github.com/sass/node-sass/pull/366. The PR is awaiting some love.

/cc @andrew et el.

[sjrdevelopment]

I'm also having this problem using gulp-sass, but I found I get a more helpful error by turning off the sourceMap and sourceComments options, if that helps the diagnosis.

[jmagnusson]

Not passing in sourceComments: 'map' "fixes" this issue for me.

[andremalkine]

I've seen this problem when using gulp-sass. I just tried out grunt-sass (grunt plugin for node-sass) and find that when sourceMap options are on, there's no error logging at all. Blatant issues pass right through unnoticed. Perhaps the scenarios are unrelated but they're strangely symmetrical.

[Arkkimaagi]

+1

I am using node-sass from gulp-sass and when I set the sourceComments: 'map' option, I get segmentation fault if there is an error in any scss file.

[am11]

This is fixed. The next release will bring tons of improvements. Don't know when it will be rolled out though. You can, however, build it by yourself if you have C++ compiler installed (also npm install -g node-gyp and follow the how-to-build instructions in repo's readme).

[keithamus]

@am11 are you saying this is fixed in master? If so I'll close this issue.

[am11]

Actually I replied to the issue @Arkkimaagi reported. That is surely fixed.

It's unclear if the OP is facing the same issue.

I guess a release at this point will close couple of issues in the tracker. Do you know of an ETA on the next release?

[keithamus]

@am11 first thing we need to do is build all of the binaries for https://github.com/sass/node-sass-binaries, updated to the latest version. I can do Mac and Linux, @LaurentGoderre usually sorts the Windows builds out. Finally, after that is done, we need to update the package.json to reflect a new version number and publish it to npm.

If you want to speed things up, feel free to try to compile the Mac/Nix/Win binaries and submit a pr in https://github.com/sass/node-sass-binaries

[am11]

Great! :+1:

Will do that, but on second thought; I guess this would make sense if we wait for https://github.com/sass/libsass/pull/471 and https://github.com/sass/libsass/pull/476 to merge in by @akhleung.

After that I will make a minor update which corresponds to https://github.com/sass/libsass/pull/476.

By that time, hopefully teletyping (TTY) test issue (https://github.com/sass/node-sass/pull/410) is sorted out, so CI build finally pass with flying colors! :green_apple:

[keithamus]

Libsass is out of my control, but feel free to @ me when they're merged in and we can get the ball rolling on updating node-sass.

[am11]

@keithamus, @andrew,
For any consumer of node-sass, is it mandatory to have python and cpp compiler installed on your system? Today I ran npm install node-sass on a system and it complained about missing python. Shouldn't we go for a production release, so there is no "building" at install-time?

[andrew]

@am11 I don't believe it should if there are binaries available for the platform you're installing it on. The build.js testing-and-installing-or-building script is kinda crazy, I wouldn't be surprised if there were bugs in it.

[am11]

@andrew thanks for the quick reply.

I think we should add a note in README.md for python and c-compiler dependencies, to clarify that the installation of this npm it's not as straightforward as npm install node-sass (the case with most of the npms).. :smile:

[nschonni]

@am11 those dependencies are only required for a rebuild of the binaries. They shouldn't be needed for anyone one a stable NodeJS on any platform the binaries are shipped for.

Closing this issue since the PR fixings this was merged weeks ago

[am11]

@nschonni, thanks! That's what I was thinking. But it was actually building the package with npm install node-sass and subsequently failing the Web Essentials build. I ran npm cache clear and it is fixed! :sunglasses:

[justinewin]

Experienced a lot of segfault errors just a while ago. Tried doing npm rebuild and reinstalling my node modules but to no luck. Finally went back to my code and noticed one line exceeded one tab. Deleted the extra tab and now it finally works! This error occured while I was testing the responsiveness of the site and made a quick change in sass and gulp watch it right ahead.

[xzyfer]

noticed one line exceeded one tab

I'm not sure what you mean by this. Can you please show an example so I can see the segfaults myself?

[justinewin]

@xzyfer take this as an example:

sass is tab-sensitive, I failed to see one line exceeded by a tab. Something similar like this:

#scroll-1
    background: url('../img/scroll-1.png')
         background-position: center
    background-size: cover

[xzyfer]

Thanks @JustineWin this should throw an error, not segfault

Inconsistent indentation: "\t     " was used for indentation, but the rest of the document was indented using 1 tab.

This is likely a bug in sass2scss /cc @mgreter

[justinewin]

@xzyfer I see, mine was because of a tab. That's why I was wondering why I'm having a segmentation fault.

[xzyfer]

@JustineWin your segmentation fault is due to an unhandled error. So yes, the extra tab is the problem, but a bug is causes libsass to segfault instead of returning an error,