Error message possible typo in node_zlib

[amarchino]

• Version: v9.1.0
• Platform: Windows 10 x64, v 1709 (OS build 16299.19)
• Subsystem: zlib

After upgrading to Node v9.1.0 (and reading the changelog. Always read the changelog) I found a possible typo in the error message introduced in #16657 .
After issuing a refresh of the dependencies (to ensure that the native modules of my project keep working), I found the message
src\node_zlib.cc:437: Assertion `args.Length() == 7 && "init(windowBits, level, memLevel, strategy, writeResult, writeCallback," " dictionary)"' failed.
The error is actually correct (one of the dependencies has a transitive dependency on an older version of node-tar, triggering the warning and the subsequent error), but it seems to me that the error message is wrong.
Should there be a comma between the quotes and the word dictionary? Or are the quotes erroneous?

Thanks for the attention.

[addaleax]

The quotes shouldn’t be there but I’d guess it’s hard to get rid of them because of how assertions are implemented in Node’s C++ layer…

[amarchino]

I am no expert regarding C++. A colleague of mine actually suggested that a possible workaround would be to add a backspace at the end of line 436 in file src\node_zlib.cc, replacing the end quote so as to make the two lines as a single one.
That is:

     CHECK(args.Length() == 7 &&
       "init(windowBits, level, memLevel, strategy, writeResult, writeCallback,\
 dictionary)");

I know that this workaround is actually horrible (stylistically speaking, at least)... But may it be a partial workaround for the problem?
As I could see, macro-wise, the obvious string concatenation is actually escaped, therefore rendering the quotes found in the actual error message.

[addaleax]

@amarchino Please feel free to open a PR with your suggestion and/or try that out if you want :)

[ChadTaljaardt]

@amarchino do you have an update on this?

[amarchino]

Sorry for the enormous delay.
I can no longer reproduce this issue via NPM (since in time NPM was updated); yet by seeing the code it seems that the problem is still there.
By searching through all the code in the 'modifiable' folders (ie: excluding /deps and /tools, as per the contribution specifications), it seems this is the only offending code.
I'll try to create a Pull Request with which to try and approach a solution. Just need the time to understand how to create it (sorry, first time working with Github).

[apapirovski]

I'm going to close this out given that the related PR was closed as non-actionable.