Removing Silent Failures from Bootstrap Autoinitialization #2315
Conversation
ghost
added
the
| DebugBreak(); | ||
| } | ||
| else { | ||
| MessageBox(NULL,L"You need to run the Windows App Runtime Installer and its MSIX components", L"MddBootstrap", 0x00000010L); |
There was a problem hiding this comment.
This is consistent with what I suggested during initial planning with @DrusTheAxe @zaryaf. This would align bootstrap-required app failures with common .NET runtime-required app failures.
I think the message should be reworded to use as much prior art as possible, so power users+ instinctively know what to do next.
Example:
There was a problem hiding this comment.
Beware without a fusion manifest, the message box will be unthemed/ugly. It may or may not get dark mode treatment angering users.
There was a problem hiding this comment.
Agreed about rewording it, we can say something like: "This application requires installing the Windows App Runtime and its MSIX components."
At least, can we add a link to the page to install it?
https://docs.microsoft.com/en-us/windows/apps/windows-app-sdk/downloads
Beware without a fusion manifest, the message box will be unthemed/ugly. It may or may not get dark mode treatment angering users.
At this point, there is neither WinUI3 nor OS XAML. So we can only delegate on Win32 dialogs, so I hope that at some points all these dialogs support dark more too.
There was a problem hiding this comment.
@riverar yes, that was my opinion too - model this as closely as possible on the precedent set for the CLR and CRT message boxes
There was a problem hiding this comment.
Let's replace "you need to" with "please verify" language. We actually don't have certainty that the WAR isn't installed here, and misdiagnosing can be even more frustrating to users.
There was a problem hiding this comment.
The title should be "[application_exe] - This application could not be started" versus "MddBootstrap" which doesn't point to the app and has little meaning for end users.
| if (IsDebuggerPresent()) { | ||
| DebugBreak(); | ||
| } |
There was a problem hiding this comment.
Opinion: I travel around a lot with the debugger and little is more annoying than hitting breakpoints I didn't set. I think an interested party would set a bp on MddBootstrapInitialize anyway, making this of no value.
There was a problem hiding this comment.
The reasoning here is that many developers are not even aware of MddBootstrapInitialize living in generated code. So knowing to set a BP on that in case of failure is a catch-22, and the problem we're trying to solve here - drawing the developer's attention to that generated call. Without the DebugBreak, we could pop the MessageBox unconditionally, but that's even more annoying to debug (break on all thread, find the one with a message pump, find the call frame ...).
There was a problem hiding this comment.
I just realized this is the consumer side, not the Microsoft impl side, so agree @Scottj1s!
| @@ -29,6 +29,12 @@ namespace Microsoft::Windows::ApplicationModel::DynamicDependency::Bootstrap | |||
| const HRESULT hr{ ::MddBootstrapInitialize(c_majorMinorVersion, c_versionTag, c_minVersion) }; | |||
| if (FAILED(hr)) | |||
| { | |||
| if (IsDebuggerPresent()) { | |||
There was a problem hiding this comment.
use "chameleon coding" - stick with original style/conventions, like brace placement
| } | ||
| else | ||
| { | ||
| MessageBox(global::System.IntPtr.Zero, "You need to run the Windows App Runtime Installer and its MSIX components", "MddBootstrap", (uint)0x00000010L); |
| else | ||
| { | ||
| MessageBox(global::System.IntPtr.Zero, "You need to run the Windows App Runtime Installer and its MSIX components", "MddBootstrap", (uint)0x00000010L); | ||
| } | ||
| global::System.Environment.FailFast(e.Message); | ||
| } |
There was a problem hiding this comment.
We could implement the C++ goo in the bootstrap DLL and then none of this C# is needed.
I'd started working on a MddBootstrapInitialize2(). Let me steal parts of this PR over into it and share
|
This PR is superseded by Rich information on Bootstrap initalization failure #2316 |
|
@ShashankNay thanks for starting this conversation. Let's run with Howard's work, which addresses all concerns with this PR. |
|
Abandoning this PR in favour of Howard's work. |
The existing
MddBootstrapAutoInitializer.csandMddBootstrapAutoInitializer.cppfiles would eitherFailFastorexitwithout giving any signal to the user. For example, if an unpackaged app's .exe was clicked without having the prerequisite Framework packages installed, it would fail silently and not let the user know why. This change adds aMessageBoxthat lets the user know that they may be missing the framework package. It will also create a debuggable moment if launched from Visual Studio.