[Windows] Exception is silently ignored when trying to load an asset with inexistent name

[ArtiomCiumac]

Environment: Windows 8.1 x64
Paradox version: 1.0.0-beta01

When a wrong asset name is specified - the exception is silently ignored and game continues to load and run, however it work as expected as not all assets are loaded.

The only clue about the issue can be seen in Visual Studio Output window:

A first chance exception of type 'System.InvalidOperationException' occurred in SiliconStudio.Core.Serialization.dll
[Scheduler]: Error: Unexpected exception while executing a micro-thread. Reason: System.InvalidOperationException: Asset [wall_bottom1] not found.
   at SiliconStudio.Core.Serialization.Assets.AssetManager.DeserializeObject(AssetReference parentAssetReference, AssetReference& assetReference, String url, Type objType, AssetManagerLoaderSettings settings, ConverterContext converterContext)
   at SiliconStudio.Core.Serialization.Assets.AssetManager.Load[T](String url, AssetManagerLoaderSettings settings)
   at Pong.PongGame.<LoadContent>d__0.MoveNext() in d:\Artiom\Projects\Pong\Pong\Pong.Game\PongGame.cs:line 34
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at SiliconStudio.Core.MicroThreading.MicroThread.<>c__DisplayClass4.<<Start>b__2>d__6.MoveNext()

It would be good to rethrow this exception on the main thread or use some other notification to let developer know about the issue as soon as possible.

[xen2]

Thanks for reporting.
We still have this ongoing discussion concerning this.

Basically MicroThreads don't currently rethrow exceptions because we treat them as "Task" and not as "Thread".

Difference is that a Thread exception will end up in global exception handler (can crash .exe if not handled), but Task won't be visibile until somebody Wait() or use Result on them, so that exception is properly thrown in right context. This is quite useful in various scenario.

What we were thinking is just change little bit the system so some "main/global" microthreads (i.e. the one that are launched with Scheduler.Run and not awaited) wouldn't be caught automatically (for example a "forwardExceptions" flag in Scheduler.Run). Basically we would allow user to choose if it behave like a Thread or a Task in terms of exception behavior.

I think that would cover most scenario well enough. Let me know if you have any feedback on that approach!

[ArtiomCiumac]

An exception, by its nature, should never happen during normal program execution - so I think any thrown exception should stop program execution or notify the developer as soon as possible in some other way.

If developer expects an exception to be thrown and intends to handle it - he should do it explicitly in his code, therefore I would prefer it to behave like "Thread" by default. Otherwise it results in unexpected behavior that is difficult to track - in my case I was getting just a blank game screen, despite my code was exactly like in the sample, just with different asset names.

[xen2]

I agree those most scripts should behave like thread in terms of exception.

However, let's say I am running something along the line of Task.Run(() => Download(file)).Wait();

If Download throws an exception, Task system won't crash the program right away (I think most people expect it not to crash, and that's the default behavior of Task system). You won't have to deal with it in UnhandledException (which has no context, everything is probably lost at that point) or wrap it in a try/catch.

However, if you wait on it, it will rethrow an exception at Wait() location (and if you don't handle it it will crash your program, which is a good thing in this case).
When starting async tasks, users usually care about exception when reading results only, because that's when you have the context to process them.

What is lacking is distinction between "main" microthreads (similar to Threads), and dependent tasks (similar to Task, which the user awaits through Scheduler.WhenAll or similar).

Maybe, to have best of both world, this distinction should be specified when starting a new microthread (with a sensible default -- or even different function such as Scheduler.RunTask and Scheduler.RunThread to clarify what happens better?).

In that scenario, I suppose mostly everything would run as a "Thread" by default, but user could start dependent microthreads, and be able to catch the aggregate exceptions at the proper location if he wants to, by doing something like

Or maybe we could consider that it is always Thread, and that async Task waiting would behave still like .NET (i.e. MicroThread shouldn't be used like async Task at all).

Note: Our microthread are neither Tasks nor Threads (in that they run cooperatively & independently like a Thread, but they are awaitable like a Task if user wants to -- that's probably only when the user awaits them that he might want the exception to be wrapped).

I will start by trying to disable exception catching and see if it is really needed somewhere.

[ArtiomCiumac]

Afaik even in Task model all exception are thrown at some point - even for fire-and-forget tasks, otherwise the program produces incorrect results.

I think we are arguing about different things here - I agree that there should be logically defined points where an exception is rethrown, however for me the most important part is that any exception which is not explicitly expected and handled by the developer should not be silently swallowed how it is now. It is much better to get an "AssetNotFoundException" (even without good context) than to see just a blank game screen ;)

I don't think we need different methods like RunThread and RunTask - it is enough to have just Run with a parameter or extension method where the developer can indicate explicitly that any exceptions should be ignored (see this for an example implementation).

[xen2]

The changes you proposed sounds good, and match what we were thinking of doing. Thanks for your input!
I will try to get that in next version. That's definitely a change that we delayed for too long (due to internal projects relying on this) and should have been in first version.

Note: Fire-and-forget tasks exceptions are swallowed with .NET framework task system.
But I agree this wasn't the best idea to try to mimic that by default with our microthreads.

[xen2]

Sorry I have noticed in the MSDN article that they are also making process crash in finalizer, didn't know about that! That definitely confirms we should handle everything the user didn't ask to explicitly ignore.

That's a behavior that can likely be done more easily than finalizers, since Scheduler knows if something await a Task or not. I'll check more in details tomorrow.

Thanks again for the insight!

[ArtiomCiumac]

I'm glad I could help!
Paradox Engine looks very well engineered - and over time it will become better and better ;)

[xen2]

Fixed in newly released beta02.