Broken on node v6.0.0

[zigomir]

Running

lessc main.less build.css

won't output any error and no build file.

Running with nodemon yields more info:

› nodemon -e less --exec 'lessc src/main.less build.css'
(node) v8::ObjectTemplate::Set() with non-primitive values is deprecated
(node) and will stop working in the next major release.

==== JS stack trace =========================================

Security context: 0x36610d2c9fa9 <JS Object>#0#
    1: .node [module.js:568] [pc=0x1aa0081d0f64] (this=0x225da61d89b9 <an Object with map 0x7af7e117be1>#1#,module=0x2666c00042f1 <a Module with map 0x7af7e1181b9>#2#,filename=0x2666c0004251 <String[136]: /Users/zigomir/.nvm/versions/node/v6.0.0/lib/node_modules/nodemon/node_modules/fsevents/lib/binding/Release/node-v48-darwin-x64/fse.node>)
    2: load [module.js:456] [pc=0x1aa008138e72] (this=0x2666c00042f1 <a Module with map 0x7af7e1181b9>#2#,filename=0x2666c0004251 <String[136]: /Users/zigomir/.nvm/versions/node/v6.0.0/lib/node_modules/nodemon/node_modules/fsevents/lib/binding/Release/node-v48-darwin-x64/fse.node>)
    3: tryModuleLoad(aka tryModuleLoad) [module.js:415] [pc=0x1aa00813899d] (this=0x36610d204189 <undefined>,module=0x2666c00042f1 <a Module with map 0x7af7e1181b9>#2#,filename=0x2666c0004251 <String[136]: /Users/zigomir/.nvm/versions/node/v6.0.0/lib/node_modules/nodemon/node_mo 1: v8::Template::Set(v8::Local<v8::Name>, v8::Local<v8::Data>, v8::PropertyAttribute)
 2: fse::FSEvents::Initialize(v8::Local<v8::Object>)
 3: node::DLOpen(v8::FunctionCallbackInfo<v8::Value> const&)
 4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
 5: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>)
 6: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)
 7: 0x1aa00800961b
 8: 0x1aa0081d0f64
(node) v8::ObjectTemplate::Set() with non-primitive values is deprecated
(node) and will stop working in the next major release.

==== JS stack trace =========================================

Security context: 0x36610d2c9fa9 <JS Object>#0#
    1: .node [module.js:568] [pc=0x1aa0081d0f64] (this=0x225da61d89b9 <an Object with map 0x7af7e117be1>#1#,module=0x2666c00042f1 <a Module with map 0x7af7e1181b9>#2#,filename=0x2666c0004251 <String[136]: /Users/zigomir/.nvm/versions/node/v6.0.0/lib/node_modules/nodemon/node_modules/fsevents/lib/binding/Release/node-v48-darwin-x64/fse.node>)
    2: load [module.js:456] [pc=0x1aa008138e72] (this=0x2666c00042f1 <a Module with map 0x7af7e1181b9>#2#,filename=0x2666c0004251 <String[136]: /Users/zigomir/.nvm/versions/node/v6.0.0/lib/node_modules/nodemon/node_modules/fsevents/lib/binding/Release/node-v48-darwin-x64/fse.node>)
    3: tryModuleLoad(aka tryModuleLoad) [module.js:415] [pc=0x1aa00813899d] (this=0x36610d204189 <undefined>,module=0x2666c00042f1 <a Module with map 0x7af7e1181b9>#2#,filename=0x2666c0004251 <String[136]: /Users/zigomir/.nvm/versions/node/v6.0.0/lib/node_modules/nodemon/node_mo 1: v8::Template::Set(v8::Local<v8::Name>, v8::Local<v8::Data>, v8::PropertyAttribute)
 2: fse::FSEvents::Initialize(v8::Local<v8::Object>)
 3: node::DLOpen(v8::FunctionCallbackInfo<v8::Value> const&)
 4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
 5: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>)
 6: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)
 7: 0x1aa00800961b
 8: 0x1aa0081d0f64

[seven-phases-max]

Less itself does not use any ObjectTemplate.Set function, so it's probably one of its dependencies. There's long chain of dependency updates in the current master so it's hard to tell yet how difficult it may be to get v6-compatible release. Feel free to investigate it yourself deeper and suggest a patch/PR.

[evanlucas]

That looks like an issue with nodemon, not less

[macinjosh32]

Seeing the same. Running lessc on any trivial .less file ouputs no build file and no errors under Node.js v6, even in verbose mode.

[jhnns]

I've found the cause:

The current published version of Less calls path.dirname() with undefined which throws an error with node v6. This has already been addressed by ec04a03 but not published yet. The error is not thrown because the current implementation of lessc swallows all errors caused by Less itself. I've created a PR to fix this.

[seven-phases-max]

So, emm, which one to merge? #2882 or #2884? :)

[evanlucas]

They fix two different issues IMO

[seven-phases-max]

So both? (I'm asking because they somewhat overlap and merging both would need some additional editing).

[evanlucas]

the other one uses process.exit() directly, which should be avoided to prevent not flushing stdout/stderr

[jhnns]

@evanlucas Can you describe how this problem with calling process.exit() directly can be reproduced? I definitely agree with you that process.exit() should be used in rare cases because usually it's better to terminate it regularly or by throwing an error. In this case, however, I think process.exit() with an error code is more appropriate, because we don't want to show a stack trace for expected errors. And it also seems weird to me to continue execution but prevent further processing with a flag (like it is currently implemented).

[evanlucas]

setting process.exitCode does not show a stack trace. process.exit() does not actually flush process.stdout or process.stderr (and they are both non-blocking). You should only have to explicitly call it in rare cases. Otherwise, close all of your handles and the event loop will close on it's own, which will cause the process to exit.

Related: nodejs/node#6409

[jhnns]

Thx for clarifying this. I did not know that process.exit() is non-blocking. I'm going to update my PR asap.

One thing though: If process.exit() did not flush process.stdout – why is there an output in my console? 😁

[evanlucas]

It is possible to have already flushed by the time the stream is closed. It just isn't a guarantee. That is why there is sometimes (partial) output in the console.

[lmeyerov]

Confirmed that, at least for us, running in head (which includes ec04a03 ), this gets resolved.

[ewebdev]

Using "--source-map" still generates an empty css file for me.. (otherwise working for me). node.js v6.0.0 on windows 8 64-bit.

[jhnns]

@ewebdev yep, I can confirm this. With #2891, it throws:

lessc --source-map-map-inline styles/main.less
path.js:7
    throw new TypeError('Path must be a string. Received ' + inspect(path));
    ^

TypeError: Path must be a string. Received undefined
    at assertPath (path.js:7:11)
    at Object.basename (path.js:1355:5)
    at /Users/jhnns/dev/jhnns/less.js/bin/lessc:292:61
    at Object.<anonymous> (/Users/jhnns/dev/jhnns/less.js/bin/lessc:486:3)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:456:32)
    at tryModuleLoad (module.js:415:12)
    at Function.Module._load (module.js:407:3)
    at Function.Module.runMain (module.js:575:10)

[jhnns]

I would be ok to open a PR that fixes this, but I'm not sure what source map options are supported in which situations, e.g. when the output is process.stdout. This code is a bit complicated and probably requires some refactoring...

[dcousens]

Confirm, this breaks with node 6. No error is output :(

[dcousens]

@matthew-dean using ec04a03 works for me (I saw it somewhere around here).

[ewebdev]

@lmeyerov @matthew-dean source map is still boken for me, both on head and on ec04a03 =/

[addaleax]

Btw, even without the error reporting bugfixes, there should be nothing that speaks against releasing a version containing my ec04a03 patch, so that lessc at least works on Node.js v6 as it did before.

[matthew-dean]

@addaleax Less 2.7.0 is released (just now). Which bugs are still outstanding? I should add that as a "known bug" to this release.

[addaleax]

That’s the only one that I have encountered, plus the “bug” of not having any real error output in the case of failure. Can’t speak for others, though.

[matthew-dean]

For some reason, source maps pass all Less tests, but after upgrading, I'm also getting the source map error, and not in a Node 6.0 environment. (Happening in Node 4.) So there's some change that has just broken source maps.

[matthew-dean]

I believe PR #2834 introduced the source map bug. Reverting commit 470af20 on my system fixed my source map issue. Can someone (@ewebdev) quickly confirm? If so, I'll commit the reversion and put out a 2.7.1 hotfix.

[matthew-dean]

//cc @nicks

[ewebdev]

@matthew-dean Reverting commit 470af20 doesn't solve the source maps issue on my env.

[nicks]

@matthew-dean sorry! it's just a performance optimization, feel free to revert if it's causing problems and we can try again later

[jhnns]

So much confusion... 😁

Since I've already put some time in it to investigate, let me clarify it:

• ec04a03 fixes a bug in bin/lessc where an undefined path was passed to dirname. This has now been published, everything's fine 👍
• There is still a bug in bin/lessc, where an undefined path is passed to basename. Just run lessc --source-map-map-inline some-file.less and less will emit nothing to stdout. There is no PR for this bug yet. I've tried to fix this, but since there are no tests for lessc and there are so many different options, I was not confident enough to not break anything else... 😞
• Both bugs were not reported to the user (e.g. like presenting a stack trace or a non-zero exit code). This is fixed by my PR which is still pending.

[addaleax]

@jhnns Oh, cool, I didn’t see that source map one.

If nothing else, one can restore the old Node v5 behaviour by changing path.basename(output) to path.basename(output || 'undefined');… I guess the whole block there can be skipped for output === undefined, though.

[matthew-dean]

@jhnns What is the effect of the bug in bin/lessc. Is it a critical bug? Does Less still complete? What is the result? Can you file a specific issue for this bug with more details?

I merged your PR for error reporting.

[claar]

@matthew-dean See #2896

[edit: Nevermind, thanks @matthew-dean]

[matthew-dean]

@ewebdev If you are still having an issue on Less 2.7.1, can you file a separate issue with more details?

@claar That is a separate issue (the first one on @jhnns's list), which has been addressed. The second item is a separate bug, and @ewebdev's is a separate bug. They need to be separated from this issue for clarity.

[matthew-dean]

@nicks It happens. You wouldn't be the first. If you can help us add tests so we can detect source maps not working in the future, that would be much appreciated.

Btw, as a general FYI, Less's dev support is a bit slim right now (one of our major contributors had to step back because of family responsibilities), so we would definitely welcome Less developers getting involved on an organizational level. You can contact me separately if you have questions.

[jhnns]

I've created a separate issue. Since the original issue has been resolved, we can close this one.

[matthew-dean]

@jhnns Sounds good. Thanks for following up.

[johngerome]

Still broke on node v6.9.2 LTS.
Nothings happen when runing lessc