Catch and rethrow the exception in StylusPlugIn #945
Conversation
this code running in Stylus Input thread and this thread is a backgroud thread that will be break when anyone throw an exception in this code
|
@lindexi is there an issue related to this? i'm trying to understand what this change is doing. From my limited understanding, this sounds like it can break apps that currently depend on how component is handling exceptions. |
...Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Common/StylusPlugin.cs
Outdated
Show resolved
Hide resolved
|
It also sounds like if we make this change in post 3.0 timeframe that the documentation will need to be updated, correct? |
Thanks @stevenbrix . And I will new a issus about it. |
…/LuniwerlurkalNearhahearnea
Yes, we should change the StylusPlugIn Class (System.Windows.Input.StylusPlugIns) document and remove this word |
Well you still would need to handle the exception in your |
|
I'm also curious to know if this is something that other community members think is a good idea. It seems reasonable to me, but I'm wondering if there are other, more full featured solutions that could possibly cover other scenarios as well. On #936, @weltkante brought up how WinForms has I'm certainly not trying to discount this effort done here, just want to make sure we are thorough and design the right thing :) /cc @dotMorten |
|
Thanks for the mention. If I understand this issue correctly, I'm concerned there's even an exception being thrown here, whether it's in the background or moved to the main-thread. IMHO the problem is still there, because I can't handle the unhandled exception (unless I put in an unhandled exception handler which IMHO is nothing but a band-aid). It's basically just an exception waiting to kill your app regardless. Instead, I think first the proper fix would be to ensure this code doesn't throw, so there's nothing to catch. I'm hoping that's possible. In the event that there isn't, it means it's an expected exception, and it should IMHO just be dealth with the right way and swallowed. At no point should an exception be thrown that the user can't directly guard against in a try/catch around a method call (since it's an event, there is nothing to try/catch). Looking at the example in the linked issue, IMHO the proper fix here is to fix the stylus plug-in. That code shouldn't throw, but just catching and swallowing it would mean the developer would never know their plugin was bad. |
Yeah this is sort of my thought, but I don't think you can always catch all
If it's in the
This is a very trivial case, it would be great to see a more complicated scenario where this makes more sense (if one exists). I also want to think of what the end-user experience is here. Do we want apps to crash just because a stylus plugin threw an exception? What if I'm writing a document with a pen and oops, the stylus stopped working, ok i'll just type. I'd prefer every WPF app be robust by default in this scenario, rather than leave this responsibility to the app developer. When apps crash like that users think it's an issue with Windows, and they are partially correct because we should be preventing scenarios like that if it's possible the app can continue functioning in a usable way. Let the user manually close the app if something becomes so broken. But I also don't know what swallowing exceptions here does, is it possible to recover the Stylus and get it to continue working? |
|
If an exception in a StylusPlugin didn't kill the Stylus thread the rest of the application would, generally, function well. There could be some erroneous behavior that is application specific, but not framework specific. For instance, if the application was using a plugin to limit stylus input to a specific area, then a failure in a plugin could then expand the input area unintentionally. This sort of application specific consequence can be handled if we maintain the ability to get the errors that are arising and react to them. Why not provide a (ThreadStatic) event in StylusPlugin for error trapping and let the application developer decide what to do with them? If the error isn't marked as handled after the event handlers are called, allow the exception to crash the Stylus thread (for compat reasons). It feels like we should be having this sort of design discussion in the issue, and not in a PR. I suggest continued discussion move over to there. |
Fair point, I'll continue the discussion there |
Adding the
try catchin RawStylusInput method that can make the WPF still running when some friend throws an exception in the StylusPlugIn method.We should not ignore any exception, so I catch it in the Stylus Input thread to make the exception do not break the Stylus Input thread and I rethrow it to the main thread.
This code can make the touch application more stabilize. But the better way is we can control the Stylus Input thread rerun.
See #1037