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

On-demand fix #108

Merged
merged 2 commits into from
Jun 13, 2022
Merged

On-demand fix #108

merged 2 commits into from
Jun 13, 2022

Conversation

fredrikhr
Copy link
Contributor

When a node.js script uses node-sp-auth and uses on-demand authentication the electron dependency might not be installed globally, but may instead simply be in the project local dependencies or even as a devDependency. This means the electron package might easily be located in a path that contains spaces. (This would also be the case if the directory npm installs global packages to contains spaces, which is common on Windows systems where the user name contains spaces).

On Windows the current code uses childProcess.executeFileSync with the shell: true option. If electron is located in a path with spaces launching the electron process fails (since the path is not quoted).

To fix this, this PR uses childProcess.spawnSync instead. This function will automatically quote arguments as appropriate for the current OS. This will also address the possibility of having the main.js file in a path with spaces or spaces in the SharePoint Site URL.

Additionally I have run into situations where electron does not work on my machine, after debugging I found the electron error message

GPU process launch failed: error_code=18

After some googling this seems to be related to electron/electron#32074. So because of this I added an instruction in main.js to add the --no-sandbox flag to the electron app and this fixes the issue.


When submitting a pull request, make sure you do the following:

  • submit the PR to the dev branch on the main repo, not the master branch
  • in the body, reference the issue that it closes by number in the format Closes #000
    Not applicable

@s-KaiNet s-KaiNet merged commit 6681acd into s-KaiNet:dev Jun 13, 2022
@s-KaiNet
Copy link
Owner

Thank you again, this PR will make the electron option more usable under different scenarios. Version 3.0.5 contains all the changes, please check whether it works well for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants