Compilation broken on win-vs2017 (only tested on Node.js14)

[mhdawson]

Looks like the CI has been broken since Nov 11th:

https://ci.nodejs.org/job/node-test-node-addon-api-new/nodes=win-vs2017/6656/console

Failure is:

  thunking_manual.cc
  object_deprecated.cc
  win_delay_load_hook.cc
  binding-swallowexcept.cc
  error.cc
  win_delay_load_hook.cc
     Creating library C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\Release\binding_swallowexcept.lib and object C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\Release\binding_swallowexcept.exp
  binding_swallowexcept.vcxproj -> C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\Release\\binding_swallowexcept.node
  binding-swallowexcept.cc
  error.cc
  win_delay_load_hook.cc
     Creating library C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\Release\binding_swallowexcept_noexcept.lib and object C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\Release\binding_swallowexcept_noexcept.exp
  binding_swallowexcept_noexcept.vcxproj -> C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\Release\\binding_swallowexcept_noexcept.node
gyp ERR! build error 
gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-v14.21.1-win-x64\node_modules\npm\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:400:28)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:285:12)
gyp ERR! System Windows_NT 6.3.9600
gyp ERR! command "C:\\workspace\\node-test-node-addon-api-new\\nodes\\win-vs2017\\node-v14.21.1-win-x64\\node.exe" "C:\\workspace\\node-test-node-addon-api-new\\nodes\\win-vs2017\\node-v14.21.1-win-x64\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild" "-C" "test"
gyp ERR! cwd C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test
gyp ERR! node -v v14.21.1
gyp ERR! node-gyp -v v5.1.0
gyp ERR! not ok 
npm ERR! Test failed.  See above for more details.
Build step 'Conditional steps (multiple)' marked build as failure
Collecting metadata...
Metadata collection done.
Notifying upstream projects of job completion
Finished: FAILURE

[mhdawson]

It's not obvious that anything was committed to node-addon-api around that time.

It does not look like 14.x changed either (last release was 4 nov).

[StefanStojanovic]

Looks like the real issue, the error is further above in the log

c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(5351): error C2672: 'Napi::details::CallJsWrapper': no matching overloaded function found (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc) [C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
  c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(5350): note: while compiling class template member function 'void Napi::TypedThreadSafeFunction<SimpleTestContext,void,0x0>::CallJsInternal(napi_env,napi_value,void *,void *)' (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc)
  c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(4940): note: see reference to function template instantiation 'void Napi::TypedThreadSafeFunction<SimpleTestContext,void,0x0>::CallJsInternal(napi_env,napi_value,void *,void *)' being compiled (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc)
  c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(5320): note: while compiling class template member function 'napi_status Napi::TypedThreadSafeFunction<SimpleTestContext,void,0x0>::Release(void) const' (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc)
  c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\typed_threadsafe_function\typed_threadsafe_function_ctx.cc(78): note: see reference to function template instantiation 'napi_status Napi::TypedThreadSafeFunction<SimpleTestContext,void,0x0>::Release(void) const' being compiled
  c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\typed_threadsafe_function\typed_threadsafe_function_ctx.cc(74): note: see reference to class template instantiation 'Napi::TypedThreadSafeFunction<SimpleTestContext,void,0x0>' being compiled
c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(5352): error C2770: invalid explicit template argument(s) for 'std::enable_if<call!=,void>::type Napi::details::CallJsWrapper(napi_env,napi_value,void *,void *)' (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc) [C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
  c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(258): note: see declaration of 'Napi::details::CallJsWrapper' (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc)

[mhdawson]

@richardlau just wondering if any of the windows auto update you mentioned were around the time we started to see failures in the job. My guess is not but better to ask.

[mhdawson]

@StefanStojanovic thanks for info above.

Looking at the blame for line 5351, it does not seem to have been modified recently.

Jun 13, 2020
implement optional function callback
details::CallJsWrapper<ContextType, DataType, decltype(CallJs), CallJs>(
env, jsCallback, context, data);

And the last change to napi-inl.h and napi.h was Oct14, 2022 so it does not seem liek the code itself was modified around the time the failure started.

[mhdawson]

Looks like the errors start at .\typed_threadsafe_function\typed_threadsafe_function_ctx.cc).

That was recently modified ! - #1201

@JckXia since you probably have the most context any chance you could take a look?

[richardlau]

@richardlau just wondering if any of the windows auto update you mentioned were around the time we started to see failures in the job. My guess is not but better to ask.

It does looks the machine was updated/rebooted on the 11 Nov (to determine, look at the build history for the machine(s) and any failing windows-update-reboot jobs (failing jobs generally mean the machines were rebooted)):

• https://ci.nodejs.org/job/windows-update-reboot/label=test-rackspace-win2012r2_vs2017-x64-1/2149/
• https://ci.nodejs.org/job/windows-update-reboot/label=test-rackspace-win2012r2_vs2017-x64-1/2150/

which led to Python errors (e.g. https://ci.nodejs.org/job/node-test-node-addon-api-new/nodes=win-vs2017/6636/console) until I rolled back and pinned Python on those machines a week ago and then the build was failing with the error in #1237 (comment).

[JckXia]

@mhdawson Looking into it now. From what I understand build https://ci.nodejs.org/job/node-test-node-addon-api-new/nodes=win-vs2017/6640/console is the earliest appearance of the error related to typed_threadsafe_function_ctx.cc , which was scheduled to run on Nov 18th, and before that there are some python errors masking the issue

My PR #1201 (comment) looked like it was merged on Nov-15th with no apparent build issues on the PR pipeline. Just curious but do we not include win-vs2017 as one of the PR pipeline stages? Will dig deeper but it was probably related to the test I've added.

[JckXia]

I think the issue may be because of the way I am instantiating TSFN and using the TSFN::New for that test. For the test, I am testing various TSFN::New overloads. A deeper look into TSFN::New shows that it passes CallJsInternal into the C NAPI napi_create_threadsafe_function.

Digging a bit more into the pipeline error messages, I saw this particular error that was interesting:

c:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\napi-inl.h(5352): error C2770: invalid explicit template argument(s) for 'std::enable_if<call!=,void>::type Napi::details::CallJsWrapper(napi_env,napi_value,void *,void *)' (compiling source file ..\typed_threadsafe_function\typed_threadsafe_function_ctx.cc) [C:\workspace\node-test-node-addon-api-new\nodes\win-vs2017\node-addon-api\test\build\binding.vcxproj]

I did some research into C2770 but only have a vague idea of what it is. Perhaps one of, or various TSFN::New invocations in the test https://github.com/nodejs/node-addon-api/pull/1201/files violated this compiler rule and it's bugging out. Or perhaps my original TSFN instantiations weren't correct for this particular compiler I think my next step is to reproduce this locally and do more digging into C2770.

[JckXia]

I was able to reproduce this issue locally using visual studio 2019 + node v12. I believe the issue might be MSVC having trouble deducting the type of call to be used with std::enable_if. I updated napi-inl.h with a workaround that seems to resolve this issue locally. https://github.com/nodejs/node-addon-api/pull/1242/files

[NickNaso]

I think that we can close the issue because it has been solved with the following PRs:

• src, test: fix errors and warnings in VS 2017 #1245
• src: refactor call js wrapper #1242