`AssertSz` doesn't actually do anything

tido64 commented 3 years ago

It seems like AssertSz is just being silently ignored. In 0.64, I hit this issue when the JS bundle is invalid. An exception gets thrown in SystemChakraRuntime::evaluateJavaScriptSimple

https://github.com/microsoft/react-native-windows/blob/392aa9ff15e58cc68b2c30db57fd57708c51756d/vnext/Shared/JSI/ChakraJsiRuntime_edgemode.cpp#L71-L72

It gets caught by ReactInstanceWin::OnError

https://github.com/microsoft/react-native-windows/blob/392aa9ff15e58cc68b2c30db57fd57708c51756d/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp#L736-L738

which eventually ends up calling

https://github.com/microsoft/react-native-windows/blob/392aa9ff15e58cc68b2c30db57fd57708c51756d/vnext/Mso/src/debugAssertApi/debugAssertApi.cpp#L79-L82

But because we don't have any listeners set, nothing happens and the exception is just ignored.

I realize this exact repro was fixed in https://github.com/microsoft/react-native-windows/commit/7b2bcbb597ae8f2dae93d4a09cef6787cdb82903, but after talking with @acoates-ms, it sounds like there are other places calling AssertSz.

Full stack trace:

>   Microsoft.ReactNative.dll!MsoAssertSzTagProc(const _MsoAssertParams & params, const char * szFormat, char * argList) Line 105   C++
    Microsoft.ReactNative.dll!MsoAssertSzTagProcInline(const _MsoAssertParams & params, const char * szFmt, ...) Line 92    C++
    Microsoft.ReactNative.dll!`void <lambda>(const Mso::ErrorCode &)'::`1'::catch$1() Line 692  C++
    [External Code] 
    Microsoft.ReactNative.dll!Mso::React::GetDefaultOnErrorHandler::__l2::<lambda>(const Mso::ErrorCode & errorCode) Line 689   C++
    Microsoft.ReactNative.dll!Mso::Details::StatelessFunctorWrapper<void <lambda>(const Mso::ErrorCode &),void,Mso::ErrorCode const &>::Invoke(const Mso::ErrorCode & <args_0>) Line 239    C++
    Microsoft.ReactNative.dll!Mso::Functor<void __cdecl(Mso::ErrorCode const &)>::operator()(const Mso::ErrorCode & <args_0>) Line 412  C++
    Microsoft.ReactNative.dll!Mso::React::ReactInstanceWin::OnError::__l2::<lambda>() Line 768  C++
    Microsoft.ReactNative.dll!Mso::Internal::ActiveObjectBase::I`nvokeInQueue::__l2::<lambda>() Line 113    C++
    Microsoft.ReactNative.dll!Mso::Details::FunctionObjectWrapper<void <lambda>(void),void>::Invoke() Line 166  C++
    Microsoft.ReactNative.dll!Mso::QueueService::InvokeTask(Mso::Functor<void __cdecl(void)> && task, std::optional<std::chrono::time_point<std::chrono::steady_clock,std::chrono::duration<__int64,std::ratio<1,1000000000>>>> endTime) Line 208   C++
    Microsoft.ReactNative.dll!Mso::ThreadPoolSchedulerWin::WorkCallback(_TP_CALLBACK_INSTANCE * __formal, void * context, _TP_WORK * __formal) Line 89  C++
    [External Code]

Environment

npx react-native --version: 5.0.1

npx react-native info:

System:
  OS: Windows 10 10.0.19043
  CPU: (16) x64 Intel(R) Xeon(R) Gold 6148 CPU @ 2.40GHz
  Memory: 4.31 GB / 15.87 GB
Binaries:
  Node: 12.16.1 - C:\Program Files\nodejs\node.EXE
  Yarn: 1.22.5 - ~\scoop\apps\yarn\current\Yarn\bin\yarn.CMD
  npm: 7.20.2 - C:\Program Files\nodejs\npm.CMD
  Watchman: Not Found
SDKs:
  Android SDK: Not Found
  Windows SDK:
    AllowDevelopmentWithoutDevLicense: Enabled
    AllowAllTrustedApps: Enabled
    Versions: 10.0.18362.0, 10.0.19041.0
IDEs:
  Android Studio: Version  3.5.0.0 AI-191.8026.42.35.5900203
  Visual Studio: 16.10.31529.105 (Visual Studio Enterprise 2019), 16.10.31529.105 (Visual Studio Community 2019)
Languages:
  Java: Not Found
npmPackages:
  @react-native-community/cli: 5.0.1
  react: 17.0.1
  react-native: 0.64.2,
  react-native-windows: 0.64.14
npmGlobalPackages:
  *react-native*: Not Found

Steps To Reproduce

git clone https://github.com/microsoft/rnx-kit.git
cd rnx-kit
git reset --hard c85e9aa2ea800dc137cbfe0d89e51ca2940e2516
yarn
cd packages/test-app
yarn build --dependencies
yarn install-windows-test-app
yarn windows

Expected Results

AssertSz should do something other than being silently discarded, e.g. throw or display a message.

chrisglein commented 3 years ago

Some additional clean up here is likely appropriate as the usage of unique assert tags isn't being appropriately used here. Direct fix is to have the assert hooked up to something appropriate rather than nothing.

chrisglein commented 3 years ago

All hits are in vnext/Mso, with one exception in vnext/Microsoft.ReactNative/ReactHost/ReactHost.cpp. @vmoroz FYI

microsoft / react-native-windows