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

Webpack crushes when tries to print a message about donating in Windows 8.1 #962

Closed
DJ-MATAIL opened this issue Jun 17, 2019 · 23 comments

Comments

Projects
None yet
@DJ-MATAIL
Copy link

commented Jun 17, 2019

What is the current behavior?

Webpack-cli crushes:

.\node_modules\webpack-cli\bin\cli.js:352

if (!e && fileOwnerId === process.getuid()) utimesSync(openCollectivePath, now, now);

TypeError: process.getuid is not a function

To Reproduce
Steps to reproduce the behavior:

  1. run npx webpack --config webpack.config.js --mode development on Monday

Expected behavior
It should not to stop a webpack process

Versions
webpack: 4.33.0
webpack-cli: 3.3.3
nodejs: 12.4.0
OS: Windows 8.1

@rseeberg

This comment has been minimized.

Copy link

commented Jun 17, 2019

When building on a windows machine, the build fails due to recent changes to the bin/cli.js
error is following:

TypeError: process.getuid is not a function
C:\Projects.....\node_modules\webpack-cli\bin\cli.js:356
if (!e && fileOwnerId === process.getuid()) utimesSync(openCollectivePath, now, now);'

It relies on process.getuid which is not available on windows.

Should probably revert the change done in comit
de41351#diff-22d5d2b57c69d2e1ac054c4d0f744ff1

Alternatively, find a way to not rely on a non windows available function.
Documentation from node
https://nodejs.org/api/process.html#process_process_getuid

@Riguidix

This comment has been minimized.

Copy link

commented Jun 17, 2019

Sounds weird but for keep in dev, i just change my date to not Monday and now it's not crashing cause of this:

if (now.getDay() === MONDAY) {

😕

@rseeberg

This comment has been minimized.

Copy link

commented Jun 18, 2019

@Riguidix
That makes sense, when looking at the offending code

const openCollectivePath = __dirname + "/opencollective.js";
const MONDAY = 1;
const SIX_DAYS = 518400000;
const now = new Date();
if (now.getDay() === MONDAY) {
const { access, constants, statSync, utimesSync } = require("fs");
	const stat = statSync(openCollectivePath);
	const lastPrint = stat.atime;
	const fileOwnerId = stat.uid;
	const lastPrintTS = new Date(lastPrint).getTime();
	const timeSinceLastPrint = now.getTime() - lastPrintTS;
	if (timeSinceLastPrint > SIX_DAYS) {
		require(openCollectivePath);
		// On windows we need to manually update the atime
		// Updating utime requires process owner is as same as file owner
		access(openCollectivePath, constants.W_OK, e => {
			if (!e && fileOwnerId === process.getuid()) utimesSync(openCollectivePath, now, now);
		});
	}
}

the inner logic is only triggered if it's a monday, and if timeSinceLastPrint is larger than six_days.
So changing your system clock would be enough to "fix" it locally for a while.

This is the old source

const now = new Date();
if (now.getDay() === MONDAY) {
	const { access, constants, statSync, utimesSync } = require("fs");
	const lastPrint = statSync(openCollectivePath).atime;
	const lastPrintTS = new Date(lastPrint).getTime();
	const timeSinceLastPrint = now.getTime() - lastPrintTS;
	if (timeSinceLastPrint > SIX_DAYS) {
		require(openCollectivePath);
		// On windows we need to manually update the atime
		access(openCollectivePath, constants.W_OK, e => {
			if (!e) utimesSync(openCollectivePath, now, now);
		});
	}
}

I would purpose something closer to that.
We are already accepting that the atime might not be updated (accessing the file could throw an error)
How about something as simple as this.

const now = new Date();
if (now.getDay() === MONDAY) {
	const { access, constants, statSync, utimesSync } = require("fs");
	const lastPrint = statSync(openCollectivePath).atime;
	const lastPrintTS = new Date(lastPrint).getTime();
	const timeSinceLastPrint = now.getTime() - lastPrintTS;
	if (timeSinceLastPrint > SIX_DAYS) {
		require(openCollectivePath);
		// On windows we need to manually update the atime
		access(openCollectivePath, constants.W_OK, e => {
			if (!e) {
				try {
					utimesSync(openCollectivePath, now, now);
				} catch() {}
			}
		});
	}
}

Is it the prettiest code ever written? Nope!
But it would at least allow windows users to build on mondays....
Alternatively, it could be simplified so that any builds on a monday trigger the message,
not just the first. Personally i would be fine with that.

If that would be the case, all of the logic could pretty much be deleted.

Thoughts?

@evenstensberg

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

Could you submit a PR?

@rseeberg

This comment has been minimized.

Copy link

commented Jun 18, 2019

Could you submit a PR?

Nope.
It would take me quite a while to get everything up and running, in order to insert a try catch, seeing as I have no idea how your project is structured, or what requirements you have for pullrequests.

Someone else who usually works with webpack-cli should be able to fix this issue in a matter of minutes.

@anikethsaha

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

@evenstensberg I think the only fix for this issue is to remove that condition fileOwnerId === process.getuid()) ,
I checked the docs multiple times and it seems we cant get the file/folder's uid in windows through node. Or we can add a check for applying that condition only if it is not in windows

@tariva

This comment has been minimized.

Copy link

commented Jun 24, 2019

Pls post stable version number

@ematipico

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2019

Latest version is the one

@BrkDalkiran

This comment has been minimized.

Copy link

commented Jul 1, 2019

Yeah, I have the same problem. I change computer time, it is working.
01.07.2019 > 02.07.2019

@LordOfDav11

This comment has been minimized.

Copy link

commented Jul 1, 2019

I got same error when I used Math.Round function. npm install or npm update doesn't fix that problem, but change computer time did.

@ematipico

This comment has been minimized.

Copy link
Collaborator

commented Jul 1, 2019

Guys, have you tried the latest version? 3.3.5?

@ext

This comment has been minimized.

Copy link

commented Jul 1, 2019

This is insane. I understand that you rely on donations to keep things floating but having this kind of logic to show a donation message is madness. Currently I'm hit by this bug and without downgrading my builds are broken until tomorrow.

Someone should really rethink if this is the most effective way to get donations (while I rethink if webpack should be kept in our toolchain).

@evenstensberg

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

I don't think that's true @ext . We initially toned down the way we outputted donation messages because people said it was annoying to output every time they install webpack (and that is what every other tool asking for donations is doing right now).

If you install the newest version, the donation banner is gone.

Let me know if there's anymore troubles...

@petvas

This comment has been minimized.

Copy link

commented Jul 1, 2019

Same on windows 10, upgrading to 3.3.5 fixed it.

@mikeRead

This comment has been minimized.

Copy link

commented Jul 1, 2019

Getting the same thing today on both 3.3.4 and 3.3.5 on windows 10

Setting the time back seems to fix it
Right now it is July 1st and changing back a day fixed it (June 30th)

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/


                            Thanks for using webpack!
                 Please consider donating to our Open Collective
                        to help us maintain this package.



                 Donate: https://opencollective.com/webpack/donate


C:\Repos\cloudflare\poc\node_modules\webpack-cli\bin\cli.js:356
                                                                if (!e && fileOwnerId === process.getuid()) utimesSync(openCollectivePath, now, now);
                                                                                                  ^

TypeError: process.getuid is not a function
    at e (C:\Repos\cloudflare\poc\node_modules\webpack-cli\bin\cli.js:356:43)
    at FSReqWrap.oncomplete (fs.js:152:20)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! cloudflare-workers-enterprise@1.0.0 build: `webpack-cli src/index.js -o dist/index.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the cloudflare-workers-enterprise@1.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\miked\AppData\Roaming\npm-cache\_logs\2019-07-01T20_05_35_682Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! cloudflare-workers-enterprise@1.0.0 dep: `set CLOUDFLARE_AUTH_KEY=133a8f100d17231a5f71b436ce8866c67440d && set CLOUDFLARE_AUTH_EMAIL=mike@sardius.media && npm run build && sls deploy`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the cloudflare-workers-enterprise@1.0.0 dep script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\...\AppData\Roaming\npm-cache\_logs\2019-07-01T20_05_35_718Z-debug.log

λ webpack-cli -v
3.3.5
@g45t345rt

This comment has been minimized.

Copy link

commented Jul 1, 2019

Updating to 3.3.5 fixed it for me. Thanks

@DJ-MATAIL

This comment has been minimized.

Copy link
Author

commented Jul 2, 2019

3.3.5 solved the problem.

@ematipico

This comment has been minimized.

Copy link
Collaborator

commented Jul 2, 2019

Closing as 3.3.5 solved the issue

@NuclearMachine

This comment has been minimized.

Copy link

commented Jul 9, 2019

based

@joepie91

This comment has been minimized.

Copy link

commented Jul 9, 2019

@prahladyeri I'm not sure what you're going on about. The point of this logic, as stated, is to reduce the frequency of the messages. Is your actual complaint here that Webpack is asking for donations at all?

Also, donations are voluntary, nothing is being "extracted".

@dbrgn

This comment has been minimized.

Copy link

commented Jul 9, 2019

This reminds me of OpenOffice Cannot Print on Tuesdays...

@prahladyeri

This comment has been minimized.

Copy link

commented Jul 9, 2019

@joepie91 Never mind, I've deleted that comment. My point is that CLI programs (especially) shouldn't be coded with such zero day if conditions which can fail on specific days. Console based programs are highly used on production web servers where such failed conditions might result in failed builds or even considerable downtime (which didn't happen in this case, thankfully). I call these conditions sneaky because its not possible to easily test their outcomes in the usual course of unit testing.

@webpack webpack locked as too heated and limited conversation to collaborators Jul 9, 2019

@evenstensberg

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

Fix: upgrade to webpack-cli v.3.3.5. I'm sorry if this has caused you harm in any way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.