TypeError: Cannot read property 'length' of null

[steelbrain]

Got this stack trace from a report filed by @rickgregory over on linter-php:

TypeError: Cannot read property 'length' of null
    at SyncWriteStream.write (fs.js:1981:38)
    at Object.execFileSync (child_process.js:471:20)
    at pathFromShellSync (/Users/rickg/.atom/packages/linter-php/node_modules/atom-linter/node_modules/consistent-path/node_modules/shell-path/index.js:52:28)
    at Function.module.exports.sync (/Users/rickg/.atom/packages/linter-php/node_modules/atom-linter/node_modules/consistent-path/node_modules/shell-path/index.js:34:11)
    at findOutPath (/Users/rickg/.atom/packages/linter-php/node_modules/atom-linter/node_modules/consistent-path/lib/index.js:27:62)
    at getPath (/Users/rickg/.atom/packages/linter-php/node_modules/atom-linter/node_modules/consistent-path/lib/index.js:17:16)
    at _exec (/Users/rickg/.atom/packages/linter-php/node_modules/atom-linter/lib/helpers.js:61:59)
    at Object.exec (/Users/rickg/.atom/packages/linter-php/node_modules/atom-linter/lib/helpers.js:152:10)

[steelbrain]

This is the line producing the error

return clean(childProcess.execFileSync(shell, ['-i', '-c', 'echo "$PATH"'], opts)) || '';

[silverwind]

I think this is a Node.js bug, likely introduced in nodejs/node@05d30d5, but I'm not sure which of those two .length accesses is the problematic one.

@rickgregory Do we know which version of Node.js is used in that particular Atom release, so we can pinpoint the exact line of the error?

cc: @ronkorving

[silverwind]

Or rather, scrap that suspicion. It's happening in SyncWriteStream.write, so the commit above is likely not the culprit.

[silverwind]

Okay more research:

https://github.com/atom/node/blob/edfbc29d09425f2f387c52d77f6351b6ce101659/lib/fs.js#L1981 is the triggering line, and I can reproduce the error in Atom with require("child_process").execFileSync("foo"). So it's just a case of a somewhat misleading error message from node.

So for this bug, it's either that process.env.SHELL is set to something weird in Atom (it is undefined in a fresh installation, which is fine), or that /bin/sh does not exist on the user's system.

[rickgregory]

Hrm. /bin/sh exists, so that's not it. More info....

A fresh atom install which then had an installer of JUST the linter and linter-php plugins reproduces this error. Uninstalling the current versions and dropping back to linter 1.10.0 and linter-php 1.1.4 works fine. Now let's do the combinations:

linter 1.10.0 and linter-php 1.1.4 works
linter 1.10.0 and linter-php 1.1.8 fails with t he above error.
linter 1.11.3 and linter-php 1.1.4 works
linter 1.11.3 and linter-php 1.1.8 fails.

Note that I can get the same failure with the SCSS linter, etc. So it seems that it's not the linter base plugin that has the issue but some component shared by the language specific linters.

[Arcanemagus]

@rickgregory Most of the linter-____ plugins utilize the atom-linter module, which in turn uses consistent-path, which uses this project to generate some data (that it then does some further messing with).

[rickgregory]

Got it. I can, if you all want, test more of the linter- plugins. Right now, that seems to be where the issue lies rather than an update to the main linter base plugin.

[steelbrain]

@rickgregory Yeah, the base linter doesn't do any linting by itself, FYI linter is an apm package, atom-linter is an npm helper module.

[silverwind]

@rickgregory I'd still be interested in the value of process.env.SHELL while you're getting that error. You can get it with the dev tools, like so:

[rickgregory]

Here ya go:

"/usr/local/bin/zsh"

[silverwind]

Does the binary it exist at that location? Any idea where this variable is being set?

[rickgregory]

Yep.

[silverwind]

Well, the only explanation I have is that something is wrong with that binary. Do these work?

require('child_process').execFileSync('/usr/local/bin/zsh', ['--version'], {encoding: 'utf8'})

require('child_process').execFileSync('/usr/local/bin/zsh', ['-ic', 'env'], {encoding: 'utf8'})

require('child_process').execFileSync('/usr/local/bin/zsh', ['-ic', 'echo $PATH'], {encoding: 'utf8'})

[rickgregory]

None of those work. All throw this error:

Cannot read property 'length' of null(…)SyncWriteStream.write @ fs.js:1981
execFileSync @ child_process.js:471
(anonymous function) @ VM477:2
InjectedScript._evaluateOn @ VM453:904
InjectedScript._evaluateAndWrap @ VM453:837
InjectedScript.evaluate @ VM453:693

[silverwind]

I wonder what's up with /usr/local/bin/zsh. Post the output of these from your terminal:

file /usr/local/bin/zsh

ls -la /usr/local/bin/zsh

/usr/local/bin/zsh --version

[rickgregory]

AHA...

/usr/local/bin/zsh: symbolic link in a loop
/usr/local/bin/zsh: Too many levels of symbolic links
lrwxr-xr-x 1 rickg admin 3 Jul 21 2015 /usr/local/bin/zsh -> zsh

Odd that this would happen now, but... that looks suspicious

[Arcanemagus]

Wow, how did that manage to get put in place lol.

[rickgregory]

Yeah... This plus the fact that it's a holiday gives me the reason I've
been looking for to do a clean install (this Mac has run betas of El
Capitan in the past) so....off to do that

On Mon, Feb 15, 2016 at 1:57 PM, Landon Abney notifications@github.com
wrote:

Wow, how did that manage to get put in place lol.

—
Reply to this email directly or view it on GitHub
#10 (comment)
.

[silverwind]

I wonder what tool installed that symlink, certainly not brew I think. 😈

Does /bin/zsh exist and work, by the way? We might be able to detect this issue through stat/lstat and fallback to it.

[rickgregory]

Yep, /bin/zsh exists.

/bin/zsh: Mach-O 64-bit executable x86_64

Actually, I wonder if I should just symlink to that. And yes, I think the
other was a brew thing. Grr....

On Mon, Feb 15, 2016 at 2:02 PM, silverwind notifications@github.com
wrote:

I wonder what tool installed that symlink, certainly not brew I think. [image:
😈]

Does /bin/zshexist and work, by the way? We might be able to detect this
issue through stat/lstat and fallback to it.

—
Reply to this email directly or view it on GitHub
#10 (comment)
.

[silverwind]

Ah, good. I don't think it was brew, rather something else. My zsh symlink is /usr/local/bin/zsh => ../Cellar/zsh/5.2/bin/zsh by the way.

[rickgregory]

OK. Oddly, I just renamed /usr/local/bin/zsh and created a new symbolic
link to that location from /bin/zsh and still get the above... VERY weird

On Mon, Feb 15, 2016 at 2:07 PM, silverwind notifications@github.com
wrote:

Ah, good. I don't think it was brew, rather something else. My zsh symlink
is /usr/local/bin/zsh => ../Cellar/zsh/5.2/bin/zsh by the way.

—
Reply to this email directly or view it on GitHub
#10 (comment)
.

[steelbrain]

@rickgregory Can you change your symlink to point to the absolute path /bin/zsh?

[rickgregory]

Oddly, I got the same issue as above. Then I screwed up with the old rm command. I have a clone of the drive before doing this, but I also did a clean install and... the issue doesn't occur. Want more info or shall we write this off as some weird edge case?

[Arcanemagus]

I'm perfectly fine with writing this off as a bizarre edge case myself, @steelbrain? @silverwind?

[steelbrain]

This is definitely an edge case, pretty much any program that relies on shell won't be able to function in such a system. Therefore I don't think this is a bug in shell-path

[rickgregory]

Yeah, the odd thing is that it hasn't seemed to affect other things on the
system and still only affects the latest versions of linter-[N] plugins
(see above that earlier versions of the plugins were OK).

Anyway, I'm fine not pursuing this if you all are OK with it.

On Mon, Feb 15, 2016 at 9:51 PM, Steel Brain notifications@github.com
wrote:

This is definitely an edge case, pretty much any program that relies on
shell won't be able to function in such a system. Therefore I don't think
this is a bug in shell-path

—
Reply to this email directly or view it on GitHub
#10 (comment)
.

[silverwind]

Okay, I'll close this in favor of sindresorhus/default-shell#2 where we might investigate a more reliable method of getting a user's shell.