Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path.join, argument[0] is coming back undefined when explicitly passed in #21392

Closed
Nantris opened this issue Jun 18, 2018 · 7 comments
Closed
Labels
process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform.

Comments

@Nantris
Copy link

Nantris commented Jun 18, 2018

Version of Node when packaging: 8.9.1
Version of Node in Electron: 8.9.3
Platform: Windows 7 Professional, x64
Subsystem: path.js
Context: Packaged Electron 2.0.2 Application

I will continue building out this report as I investigate.

On the surface, it appears that this setup where each argument to path.join is defined results in argument[0] undefined if I step into path. I've been trying to debug this all day and finally am at the point where I believe this could be a Node related bug.

Call to path.join from NPM package
image

Literally as soon as you step into path.js you find that argument[0] does not contain C:\Windows\ as you would expect, but instead it is undefined.

image

This leads to a failure at assertPath almost immediately.
image

I am going to upgrade NodeJS to 8.11.3 and repackage, and see what the outcome is.

@addaleax
Copy link
Member

I think this might have been fixed by #18463, upgrading Node seems like a good candidate for resolving this (and even if not, please do it because you want to keep up to date with security releases).

Nantris pushed a commit to Nantris/node-winreg that referenced this issue Jun 19, 2018
Hardcoded path.join first arg to try and get this working. Cause of bug may be nodejs/node#21392
Nantris pushed a commit to Nantris/node-auto-launch that referenced this issue Jun 19, 2018
Changed to a custom implementation of winreg to try and work around nodejs/node#21392
@Nantris
Copy link
Author

Nantris commented Jun 19, 2018

This appears to be unrelated to #18463.

I just went back to Electron 1.7.15 and I'm having the same issue even there with Node 7.9.0. According to the Node team, this bug didn't affect versions of node prior to 8.x.x

@ChALkeR
Copy link
Member

ChALkeR commented Jun 19, 2018

@slapbox

  1. What is/was the exact location of the path.js file shown on the last screenshot in Path.join, argument[0] is coming back undefined when explicitly passed in #21392 (comment)? Just to be sure that it is coming from Node.js.
  2. What does console.log(process.env.windir) print from registry.js? If it is undefined, this probably has nothing to do with path module.
  3. How is process.env.windir set? Could it happen that it is set after the function is called?

@ChALkeR ChALkeR added windows Issues and PRs related to the Windows platform. process Issues and PRs related to the process subsystem. labels Jun 19, 2018
@bzoz
Copy link
Contributor

bzoz commented Jun 21, 2018

Sorry, I can't reproduce this:

> process.version
'v8.9.1'
> process.env.windir
'C:\\WINDOWS'
> require('path').join(process.env.windir, 'system32', 'reg.exe')
'C:\\WINDOWS\\system32\\reg.exe'
> process.env.windir
'C:\\WINDOWS'

@Nantris
Copy link
Author

Nantris commented Jun 21, 2018

@bzoz thank you for trying. I also tried to reproduce, but so far cannot. Specific version that runs in Electron is 8.9.3, I will update the version above. I don't think this can be easily reproduced though. This must be the edge case of edge cases.

@Nantris
Copy link
Author

Nantris commented Jun 21, 2018

I eventually hardcoded this value. Really I should have done something like: return \${process.env.windir}\system32\reg.exe`to cover cases where the OS drive is notC:`.

In any event, I've given up on resolving this issue specifically and just hoping it resolves in some future version of Node.

@jasnell
Copy link
Member

jasnell commented Jun 25, 2020

Definitely think we can close this.

@jasnell jasnell closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

5 participants