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

UncaughtException on Windows runner #21

Closed
willwray opened this issue Nov 13, 2022 · 12 comments
Closed

UncaughtException on Windows runner #21

willwray opened this issue Nov 13, 2022 · 12 comments
Assignees

Comments

@willwray
Copy link
Contributor

(Works for me on ubuntu-22.04 and macos-12 github runners, thanks.)

Fails on windows-2022 and windows-2019 runners with:

[Windows 2022]
Run de-vri-es/setup-git-credentials@v2
node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^

[Error: EPERM: operation not permitted, ftruncate] {
  errno: -4048
  code: 'EPERM',
  syscall: 'ftruncate'
}
@de-vri-es
Copy link
Owner

Interesting. This looks like it may be a problem with nodejs on Windows. But maybe we can avoid or fix it on our side. Do you have a full strack trace maybe? That would help a lot.

@willwray
Copy link
Contributor Author

Rerunning the failed action with 'enable debug logging' set doesn't give any more output than posted above
(in fact, the error seems to stop any stacktrace)

This may be useful:
https://developer.ibm.com/blogs/nodejs-15-release-blog/#updated-handling-of-rejections

You can avoid these warning messaging by handling the rejection with a catch block:

new Promise((resolve, reject) => {
reject('error');
}).catch((error) => {});

(A search on the error message has many similar hits, with similar conclusions)

@willwray
Copy link
Contributor Author

The error message Error: EPERM: operation not permitted, ftruncate
implicates lack of permission for the file.truncate

https://github.com/de-vri-es/setup-git-credentials/blob/main/src/main.ts#L30-L31

// Replace the entire file, so it doesn't matter if it ended with a newline before.
file.truncate(0);

@willwray
Copy link
Contributor Author

Found a clue here nodejs/node#3177

I thought that as long as you have the appropriate privileges and the file is opened for writing (not appending), you should be able to truncate it in either direction.

that was the problem. I had the file opened on Windows using "a+".

https://github.com/de-vri-es/setup-git-credentials/blob/main/src/main.ts#L26

const file = await fs.open(`${xdg_config_home()}/git/credentials`, "a+", 0o600);

Try opening with "w" write permissions?

@de-vri-es
Copy link
Owner

de-vri-es commented Nov 13, 2022

Ah, good point. The truncating was added later, but I forgot to update the file mode. Do you want to submit a PR to fix it? If not, I will fix it hopefully early this week.

Either way, thanks for the report and the additional information. I'm pretty sure you pinpointed the problem :D

@willwray
Copy link
Contributor Author

Thanks; this is a bit out of my wheel house so I'm happy to wait for a fix if and when you try it.
If it works on your cases without regression then I'll test it on my Windows actions and report back.

@de-vri-es de-vri-es self-assigned this Nov 13, 2022
@de-vri-es
Copy link
Owner

Al-right, that sounds good! Thanks in advance for the testing. It will help a lot, since I don't have access to a windows machine.

@de-vri-es
Copy link
Owner

I've released 2.0.10 which no-longer truncates files, but simply only adds credentials that weren't already present.

Since there's no truncate anymore, I think this will also fix the problem on Windows.

@willwray
Copy link
Contributor Author

Thanks for the quick turnaround. I confirm that 2.0.10 fixes the issue on my windows runner.

btw, it should be straightforward to add windows runners to the new unit tests, by adding stategy: and matrix: levels to the workflow .yml, and with os: [ubuntu-latest, windows-latest] list:
https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs

Perhaps I should PR that.

@de-vri-es
Copy link
Owner

Awesome! Thanks for testing.

And a PR to test on windows would be most welcome. I'm also using a few POSIX command line utilities for the tests though, not sure how easy it would be to write that in a cross-platform way.

@willwray
Copy link
Contributor Author

PR #23 sketches a sufficiently cross-platform way - specifying shell: bash.
The windows runners execute on WSL which should be reasonably cross-platform
(the PR did, however, hit an issue with windows-vs-unix line endings
and we had already encountered the ftruncate difference here).

(In fact, unaware that the default shell is powershell, my github actions were already
running fine on Windows, using mesonbuild to test c++ projects)

@de-vri-es
Copy link
Owner

Cool, that saves us a lot of hassle. Thanks for the PR, and thanks again for the reporting, debugging and testing :D

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

No branches or pull requests

2 participants