Reported percentage is lower than the last

[revelt]

Recently there were updates somewhere and suddenly one of my CLI's stopped working because p-progress threw an error complaining that the reported percentage is lower than the last reported:

p-progress/index.js

Lines 78 to 86 in a24163f

	if (progress === this._progress) {
	return;
	}
	if (progress < this._progress) {
	throw new Error('The progress percentage can\'t be lower than the last progress event');
	}
	this._progress = progress;

I edited p-progress right in node_modules and simply commenting out the throw solves the problem, that case progress < this._progress can be ignored:

p-progress/index.js

Line 83 in a24163f

throw new Error('The progress percentage can\'t be lower than the last progress event');

Idea

What if we changed it to do nothing if no bigger progress is reported (no matter it's smaller or equal than previous):

if (progress <= this._progress) {
	return;
}

this._progress = progress;

App would just skip unacceptable values until a bigger value is reported. Currently, API seems too strict.

What do you think?

[sindresorhus]

Why are you giving it smaller values than previously? It's always easier to come up with an elegant solution when I know your use-case instead of just the problem.

[revelt]

Why are you giving it smaller values than previously?

To my best understanding, smaller values should not come, I'm p-mapping over pacote's results... It's definitely not intentional :)

[timlesallen]

@sindresorhus I'm hitting this same problem, running in a browser, generally after you tab away and come back. My best guess is that event handlers are all firing at once when you come back to the tab, and that order is not guaranteed. I would vote to remove the throw (and just not update the value) here as it's messing with our exception reporting (I can't really find away to catch these as they are out of context).

Loving p-progress - thanks!

[sindresorhus]

To my best understanding, smaller values should not come, I'm p-mapping over pacote's results... It's definitely not intentional :)

Are you using the index argument? If so, that's definitely wrong if p-map is run without setting the concurrency option to 1.

[sindresorhus]

I kinda regret the "passing in percentage" interface. It would have been better to accept a totalUnitCount upfront and then have it accept a unitCount parameter. Then there would be no chance of getting a regressed percentage as it would be calculated for you.

[revelt]

Are you using the index argument?

No.

@sindresorhus if you feel uneasy about MR #9 feel free to close this issue and that ninth PR, no hurt feelings. I'm grateful for all your libraries, let's concentrate on features and serious issue tickets instead. Feel free to close this issue.

[sindresorhus]

No-no, I'm fine with lifting that restriction. You're right it's not important enough to reject the promise.

[sindresorhus]

Regarding #8 (comment). I posted some thoughts in #1 (comment).