Microsoft-ui-xaml: Discussion: Non-existing attributes or resource values should not crash a UWP app

Created on 6 Sep 2019  ·  16Comments  ·  Source: microsoft/microsoft-ui-xaml

This week, our company had a bad experience of pushing an update that causes different crashes with older versions of Windows 10 like 17134 and 16299.

Discussion: Non-existing attributes or resource values should not crash a UWP app

The main issue is Visual Studio does not do an accurate job to warn the developer when an attribute or resource value is not supported on previous Win 10 versions (see https://github.com/microsoft/microsoft-ui-xaml/issues/1285
https://github.com/microsoft/microsoft-ui-xaml/issues/1287
https://github.com/microsoft/microsoft-ui-xaml/issues/1288). I'm pretty sure there are multiple other examples if editing different controls.

What would be the harm if at runtime an attribute would be ignore if not recognized? The same can be applied for an invalid resource value. The attribute could just not apply the value.

bug team-Markup

Most helpful comment

Let's go back to basics.

XAML is provided by the developer to the framework to render.
This XAML is equivalent to user provided content. (It comes from a person using the software, not someone who wrote the software.)

There should never be anything that someone can input to your software that causes it to crash!

I think it's fair to call the current behavior a bug.

What should happen:

  1. Catch issues at compilation/build time when possible.
  2. Do not crash if something invalid gets through step 1. (Remembering that XAML can be parsed and compiled at run time and may be dependent upon where it runs.)
  3. Report or log whenever step 2 catches something. Where and how this is logged should be configurable and be available to the developer of an app.

Related
There should be a simple, well documented way of testing a UWP app running on different versions of Windows. Relying on maintaining different Virtual Machines for each release is slow, complicated, and error prone.

All 16 comments

Personally I'd want a compile error. The main harm I see here is that I have no idea why the text on my TextBlock isn't working because I'm obviously setting the Taxt property (how often have you wasted hours due to a simple typo?). Swallowing obvious errors should not happen. I want to know that what I'm doing is wrong before I even run it. In addition the class properties are essentially the xmlns schema so it would violate the schema.

However you bring up a second problem here: backwards compatible xaml. That's the issue that should be addressed directly. Again I want errors that I'm using features not available on an older platform without guarding against it.

Generally I set my Min and Max versions to be the same to avoid this mess, but unfortunately WinUI forces you to target 1903, which puts me in this danger zone. It's incredibly annoying.

I don't think errors is a good solution for many reasons:

  • Some pages of the app are only accessible to a subset of supported version

    • For example, a page only accessible when you run on RS3+, because depending on a new RS3 feature, an error should not happen in this case.

  • Apps can use different control templates and brushes for pre-Fluent and post-Fluent (starting Win10 FCU with Acrylic, Reveal brush, etc...)
  • A page can be only used by some Windows families that don't even support MinTargetVersion (Hololens doesn't care about TH1, TH2, for example).

I would recommend a mix of both of your ideas and supporting the examples above:

  • If a property is not part of TargetVersion -> fail the compilation
  • If a property is part of TargetVersion but not supported by the running OS version -> compile, run, but ignore this property.

This solution will solve @dotMorten's issue (Taxt will block the compilation) but still improve the backward compatibility.

@rudyhuyn You missed the important last part of this sentence:

I want errors that I'm using features not available on an older platform without guarding against it.

We already have that. It's a matter of tagging an element to require a certain feature. So for entire pages you'd just tag the root element of the page.

In any case the backwards compat issue should be a thing of the past by v3.0, and I'm guessing a feature like discussed here wouldn't happen until then anyway.

Let's go back to basics.

XAML is provided by the developer to the framework to render.
This XAML is equivalent to user provided content. (It comes from a person using the software, not someone who wrote the software.)

There should never be anything that someone can input to your software that causes it to crash!

I think it's fair to call the current behavior a bug.

What should happen:

  1. Catch issues at compilation/build time when possible.
  2. Do not crash if something invalid gets through step 1. (Remembering that XAML can be parsed and compiled at run time and may be dependent upon where it runs.)
  3. Report or log whenever step 2 catches something. Where and how this is logged should be configurable and be available to the developer of an app.

Related
There should be a simple, well documented way of testing a UWP app running on different versions of Windows. Relying on maintaining different Virtual Machines for each release is slow, complicated, and error prone.

IMHO we just need to do what we do in C#. XAML is after all really just a different way of setting up UI that you could have done with C#.

In C# we have APIs that can guard against using older APIs on newer platforms and we have analyzers that can check whether we guard their use. Why can't we just have that on xaml?

In C# we have APIs that can guard against using older APIs on newer platforms and we have analyzers that can check whether we guard their use. Why can't we just have that on xaml?

The feature exists, it's called Conditional XAML. The biggest problem is that VS doesn't inject these automatically for you when copying the templates in. I think that would be the most straightforward mitigation.

  1. Catch issues at compilation/build time when possible.
  2. Do not crash if something invalid gets through step 1. (Remembering that XAML can be parsed and compiled at run time and may be dependent upon where it runs.)
  3. Report or log whenever step 2 catches something. Where and how this is logged should be configurable and be available to the developer of an app.

There's a hidden 4th one here too:

  1. Be able to catch and recover from exceptions when adding something to the VisualTree that is invalid and not caught in the 3 steps above...

All these are vital to something like XAML Studio where the point is to provide a round-trip experience for rapid prototyping, but any of these issues above ends up taking down the app, as I have no way as the developer to catch and recover from these types of exceptions.

Related
There should be a simple, well documented way of testing a UWP app running on different versions of Windows. Relying on maintaining different Virtual Machines for each release is slow, complicated, and error prone.

👍 I wonder if the new sandbox experience could help with this somehow and be able to just launch it in a different windows configuration from VS like we would deploying to a different Android emulator version. That'd at least be a better compromise as those are easy to setup, configure, manage, and use rather than manually having to setup a new VM in Hyper-V for every version you want to test.

This would be especially useful with the added CornerRadius property on many controls. In some cases I had to restyle the button radius to 0 where the new radius doesn't work (application specific). However, the CornerRadius property is not supported on older versions... Therefore, my styles would crash the app on older versions of Windows. Instead of redoing a lot of styles from scratch I just upping the TargetPlatformMinVersion to 18362; however, that requires end users to update Windows as we all know.

I look forward to WinUI 3 where this issue should go away because the whole UI stack ships with the App. However, more fundamentally, I 100% agree that the App should not crash on a missing resource or even a missing Property in a Style Setter at runtime. Compile-time checks are great but at runtime this should behave a lot more like HTML/CSS.

@jevansaks not sure if it should be a new issue or related to this one. But the error messages from the xaml parser are always vague (like they're being caught and re-trown as something generic).

For instance, I had an invalid template binding name (or maybe it was a static resource reference) in the default style for a templated control. So I get an error at runtime of No matching constructor found on type X. at the app unhandled exception handler (even with all the exceptions on), the stack trace is all just the error reporting stack. (I can even hit a breakpoint I set in the constructor, so the message makes no sense). Sometimes you get a line number, but that's actually indexed into the resource dictionary file instead (which is the only hint of what's wrong). Instead it should be saying like "Invalid resource of name X not found in file Y. [line ...]" to make it super clear to the developer where to go look.

Regarding the original issue - "why don't you get a warning when member X on type Y exits in Target Platform Version (TPV), but not in Target Platform Min Version (TPMV)", I see that we do have a warning, WMC0152, and I remember seeing that warning in the past. It looks like it no longer works. I'll look to see why is that - I have a repro using the CornerRadios property. Such warnings would be annoying, but an easy fix would be to add a conditional namespace on the offending member (or type) and it would go away. We don't check that the conditional you placed is right (because we can't), but its presence would tell the compiler that you are aware of that API missing from TPMV. I'll get back with an answer on why that's broken soon.

The second issue is "why do we not have more compile time checks on expressions where we can determine that a runtime exception on evaluation of such expression will bring the app down". One such example is {Binding} expression typos. The answer to that is that we're looking at making that happen by improving the XAML parser validator.

Our errors may be a little (or more) misleading, too. We'd love to address all those on a case by case basis - please open a concrete issue with a XAML sample. We're also looking forward to open sourcing WinUI 3 so that such a fix will be just a matter of accepting a community contribution, and not tight to our team's internal resources.

(I'll be back with an answer on the first issue)

Ok, just checked and the warning does work. If I compile this Xaml:

<Button CornerRadius="3" />

in an app targeting 10.0.18362.0 with 10.0.17134.0 as TPMV, the compiler correctly identifies CornerRadius as being introduced in version 7 of IControl and fires the following warning:

MainPage.xaml(12,17): XamlCompiler warning WMC0151: Member 'CornerRadius' on type 'Windows.UI.Xaml.Controls.IControl7' is defined under contract 'Windows.Foundation.UniversalApiContract' version '7.0.0.0', but the contract version for the targeted min version is '6.0.0.0'!

To make the warning go away, I declared this conditional namespace on the page:

xmlns:c7="http://schemas.microsoft.com/winfx/2006/xaml/presentation?IsApiContractPresent(Windows.Foundation.UniversalApiContract,7)"

and tagged CornerRadius with it:

<Button c7:CornerRadius="3" />

If I'm misunderstanding your scenario, please correct me and send me a quick XAML sample I can run on my end.

Dumb question: If the compiler can detect that CornerRadius isn't available, why can't it just conditionalize it for me instead of forcing me to deal with it?

If I ignore my warnings, I would be building an app that works great on my machine, but magically crashes on other people's older Windows PCs.

Sure you could still warn me so I know this won't have an effect, but I'm sure we can all agree, if I do overlook this, it'd be better my app doesn't have a corner radius, than it crashing.

I'm sure you could find cases where automatically conditionalizing it wouldn't be great either, but I'd make the argument that on average it's better to do nothing and continue to run, than the other edge cases where something won't be working fully as expected (ie guaranteed crash vs probably works fine).

@danzil Thanks for jumping in. Adding the CornerRadius directly work, however please follow my steps:

1- Create the project but target the minimum to 16299
2- Add the Button
3- Edit template the Button

You will notice that VS detects well the BackgroundSizing property that is invalid on 16299. If you remove the 3 usages of this property, you will have 0 warning. However, you can still see the CornerRadius from the template.

image

@ArchieCoder got it now! Thanks! That looks like a bug to me. It looks like we don't look at the control template for some reason. For me, VS does not warm me either.

@dotMorten the markup compiler does not have a good way to make code changes in a way that the user would not be spooked about it. But one component that could suggest and/or make such changes is the VS intellisense/language service in the XAML designer/editor. I think it already tries to do several things about these APIs (when it correctly detects them - my example not Archie's), and that is:

  • intellisense does not offer APIs when Min SDK does not define them (types or members)
  • if you go against it and type it anyway (or copy/paste it), it squiggles it in blue

What it would be great if it also did, would be to offer a "Quick action and refactoring" light-bulb that will conditionalize it for you - like what you're suggesting. And that action should be easier to invoke - I think the shortcut is Ctrl + .

So, I'll turn this into a bug to fix the markup compiler. I'll also open 2 more issues for the intellisense folks in VS to:

  • add support for conditionalizing APIs
  • for this control template case where they don't warn you in intellisense

the markup compiler does not have a good way to make code changes

Yet ;-)

IMHO it should either be a compilation error or it should not be a runtime error (auto-conditionalized). Getting caught out by these sort of issues after deployment is not a good developer experience - especially when they can be so tricky to reproduce (I get you as a developer could guarantee that a certain page would never get loaded on an old OS, but then you could conditionalize the entire page to hint the compiler that it's "ok").

Just had to spend another chunk of time debugging another similar scenario. I moved a style from within a control to outside as part of a resource dictionary. Within that style was a VisualStateManager which had the following XAML defined:

    <ObjectAnimationUsingKeyFrames Storyboard.TargetName="ContentBorder" Storyboard.TargetProperty="FocusVisualSecondaryBrush">
        <DiscreteObjectKeyFrame KeyTime="0" Value="{Binding Converter={StaticResource ContrastBrush}, ConverterParameter={ThemeResource TextControlForeground}}" />
    </ObjectAnimationUsingKeyFrames>

However, the converter was a local resource in the original spot, so now ContrastBrush wasn't a valid resource. This was crashing at runtime in the measure/layout step now as the VSM couldn't find the resource for the converter for the binding. Had to go back to trusted and true "comment out XAML until it works" debugging approach to find this. ☹

If at any point a resource can't be found it should be ignored (and there should be some debug log/warning about it at least in the output window, if not tie this to the new binding output window for resource key failures too), it should never crash!

It's also helpful in these cases to know if the original binding target was found but then the converter wasn't or if the converter itself had an exception, etc... as otherwise it's just a mystery.

Attached the stack trace from this scenario: VisualStateCrash.txt

Was this page helpful?
0 / 5 - 0 ratings