Limited exception information ... stuck ...

[TehWardy]

Hey guys,

I got everything running under .Net 4.x and then it turned out for M$ reasons the only way everything was going to work the way I wanted was to upgrade to .Net Core 2.2 so here I am.

This code works perfectly under .Net 4.x but for some reason under .Net Core I can't seem to get it to behave and the exception is giving me literally nothing.

So here's the code ....

var script = "((Core.Objects.Workflow.Activities.DebugActivity)activity).Message = ((Core.Objects.Workflow.Activities.Start<System.Object>)activity.Previous[0]).Data.Message;";
var result = assignContext.Compile<Action<Activity>>(script, "activity");

At runtime I have a collection of Activities that derive from this base type ...

   public abstract class Activity
    {
        public string Ref { get; set; }
        public Activity[] Previous { get; set; }
        public Activity[] Next { get; set; }
        public Action<Activity> AssignValues { get; set; } 
        ....
    }

... I have a "Flow" object that contains a collection of activities and a collection of "Links" which contain the expressions that I need to compile.

After constructing each activity as it's concrete type and assigning each of the previous and next arrays shown in the base type above I run the code that's failing in an attempt to build the AssignValues Action for each activity that has link information.

The exception information I'm getting looks like this ...

Message

Value cannot be null.

Stack trace

   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at .(ExpressionScope , SyntaxNode , StringBuilder , Int32& , Boolean )
   at .(ExpressionScope , SyntaxNode , StringBuilder , Int32& , Boolean )
   at .(ExpressionScope , SyntaxNode , Boolean )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at .(ExpressionScope , SyntaxNode )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at .(ExpressionScope , SyntaxNode , Expression , Boolean )
   at Z.Expressions.CodeCompiler.CSharp.ExpressionParser.ParseSyntax(ExpressionScope scope, SyntaxNode node, Type resultType)
   at .[](EvalContext , String , IDictionary`2 , Type , EvalCompilerParameterKind , Boolean , Boolean )
   at Z.Expressions.EvalContext.Compile[TDelegate](String code, String[] parameterNames)
   at Workflow.FlowInstance.BuildAssign[T](T activity, Flow flow) in D:\Core Prototype\Everything\Workflow\FlowInstance.cs:line 247

Is there any obvious reason that this would fail as this seems pretty straightforward for the code I have?

[TehWardy]

Oh one other thing ... this is how I setup the "assignContext" which is a ZZZ EvalContext that i'm using for this call ...

// some of the stack might not have been loaded, lets make sure we load everything so we can give a complete response
// first grab what's loaded
var loadedAlready = AppDomain.CurrentDomain
        .GetAssemblies()
        .Where(a => a.FullName.StartsWith("Core"))
        .ToList();

// then grab the bin directory
var binDir = Assembly.GetExecutingAssembly().CodeBase
    .Replace(Assembly.GetExecutingAssembly().CodeBase.Split('/').Last(), "")
    .Replace("file:///", "")
    .Replace("/", "\\");

// from the bin, grab our core dll files
var stackDlls = Directory.GetFiles(binDir)
    .Select(i => i.ToLowerInvariant())
    .Where(f => f.EndsWith("dll") && f.Split('\\').Last().StartsWith("core"))
    .ToList();

// load the missing ones
foreach (var assemblyPath in stackDlls)
    if (loadedAlready.All(a => a.CodeBase.ToLowerInvariant() != assemblyPath))
        loadedAlready.Add(Assembly.LoadFile(assemblyPath));

// now everything is loaded, lets not do this again
stackAssemblies = loadedAlready.Distinct(new AssemblyComparer()).ToArray();

assignContext = new EvalContext { IncludeMemberFromAllParameters = false };
assignContext.RegisterAssembly(stackAssemblies);

... all the types I care about are in assemblies named "Core.*.dll" so this is a targeted fetch that pulls all the things it could possibly need.

[JonathanMagnan]

Thank you @TehWardy for reporting,

We have not been able to reproduce it but we might have missed something.

Do you think you could provide a standalone version with only this code that fails?

[TehWardy]

New project > net core console app, then drop this in ...

using System;
using Z.Expressions;

namespace ExpressionThing
{
    class Program
    {
        static void Main(string[] args)
        {
            var script = "((ExpressionThing.DebugActivity)activity).Message = ((ExpressionThing.Start<System.Object>)activity.Previous[0]).Data.Message;";
            var ctx = new EvalContext { IncludeMemberFromAllParameters = false };

            var victim = new DebugActivity {
                // Assigned by unrelated code to a collection of stuff that inherits Activity.
                Previous = null, 
                Next = null,

                // The problem code
                AssignValues = ctx.Compile<Action<Activity>>(script, "activity")
            };

            Console.ReadKey();
        }
    }

    public class DebugActivity : Activity
    {
        public string Message { get; set; }
    }

    public class Start<T> : Activity
    {
        public T Data { get; set; }
    }

    public abstract class Activity
    {
        public string Ref { get; set; }
        public Activity[] Previous { get; set; }
        public Activity[] Next { get; set; }
        public Action<Activity> AssignValues { get; set; }

    }
}
}

My understanding is that this should work as i'm only trying to compile the function at this time and not actually run it.

[TehWardy]

@JonathanMagnan random question ...
Do you have a discord server?

I use discord a lot and we have a discord server for the company, it's real easy to share code samples and discuss problems on.

[JonathanMagnan]

Thank you,

We are currently looking at it.

[JonathanMagnan]

Hello @TehWardy ,

You do not register class in your project which causes the issue.

Here is a way to do it by registering a namespace:

ctx.RegisterNamespace(typeof(Program).Assembly, "ExpressionThing");

[TehWardy]

Oh ... that's new ... I didn't have to do that before under .Net 4 ... is this a .Net Core thing?

[TehWardy]

Given my original example source the last line of code had ...

assignContext.RegisterAssembly(stackAssemblies);

.... Is it possible to have an override of this that auto registers all namespaces in the given assmeblies?

[JonathanMagnan]

Hello @TehWardy ,

No, that's not new to .NET Core. You had to do it also before.

Is it possible to have an override of this that auto registers all namespaces in the given assmeblies?

I'm not sure to understand what exactly you want. You just need to call the RegisterAssembly. If you have a class that inherit from our context, you can add this code in your constructor.

[TehWardy]

My understanding is that if I do this ...

var ctx = new EvalContext { IncludeMemberFromAllParameters = false };
ctx.RegisterAssembly(stackAssemblies);

I'm constructing the context then telling it what types I want it to know about as a collection of assemblies that contain them.

What I think you're saying is that In addition to providing the assemblies I also need to tell it what namespaces from those assemblies I want to use.
What I guess i'm asking is "Given either a collection of assemblies or defaulting to the current AppDomain can the context simply know about all types in all namespaces".

What's the purpose of the RegisterNamespace call beyond what I would hope is default behaviour based on what I have described?

[JonathanMagnan]

What I think you're saying is that In addition to providing the assemblies I also need to tell it what namespaces from those assemblies I want to use.

Not at all,

The RegisterAssembly(stackAssemblies); works great. Our library will register all types in all namespaces which is exactly what you want.

You do not have to use the RegisterNamespace, it is just another way to register type in our library.

[TehWardy]

Oh i see what you're saying ... yeh my bad ...
That means the sample I gave you is broken in a different way to the code in my main code base but exhibits the same exception behaviour so the exception could be related / similar to the actual problem I have.

Having checked in my code and the expression used when I put a breakpoint on the last line in the original question example I find that the assembly collection passed in contains the assembly with both types I'm using in the expression ... do I also need to provide thing's like System or is the framework automatically included?

I'm thinking the smart way to go about this is to simply load all assemblies I care about in to the app domain then just give that last line the full set like this ...

var ctx = new EvalContext { IncludeMemberFromAllParameters = false };
ctx.RegisterAssembly(AppDomain.CurrentDomain.GetAssemblies());

... Would you expect that to behave as expected?
The trouble is that exhibits the same behaviour.

Is there some way to query the context to ask it for a list of types it knows about so I can watch window it and double check that I have everything setup as expected?

[JonathanMagnan]

Hello @TehWardy ,

Just to let you know, my developer started to work on something that will tell you which types are missing.

He almost completed the code so we should be able to provide you something probably on Monday.

[TehWardy]

Awesome :) you guys are on overtime bashing this out on a bank holiday weekend I love the dedication :)

[JonathanMagnan]

Hello @TehWardy ,

Just to confirm we made the right thing, how do you expect this functionality to work?

• Should it tell you all types used?
• Should it tell you only types missing?
• Should it auto add missing types? (might not always work if multiple type names are in different assembly)

[TehWardy]

For the above ...
It would be really good to have a method like ...

Type[] types = context.GetKnownTypes();

... which lists all registered types that the parser can consume (including ideally, empty generic types).

Ideally I would write my expressions, give them to the parser and then the parser would give me back (in the event of an axception) saying something like "unknown type used for cast" or "cannot resolve a property on the type in expression where expected type is {type name}".

Something along those lines.

Also having the ability to say to the parsers context "Get me this type({some string like a cast expression string})" to see if it can resolve a given type would be really helpful.

[JonathanMagnan]

Hello @TehWardy ,

We added UsedTypes, MissingTypes, RetryAndFindMissingTypes to check what type has been used during the last compilation

Here is an example:

var context = new EvalContext();
context.UnregisterAll();
context.RetryAndFindMissingTypes = true;

var compiled = context.Compile<Func<object>>("new SqlConnection()");
var usedTypes = context.UsedTypes;
var missingTypes = context.MissingTypes;

Let me know if that's what you were looking for.

Best Regards,

Jonathan

[TehWardy]

sweet so if I do this will it tell me where I messed up ...

var context = new EvalContext();
try
{
      var compiled = context.Compile<Func<object>>("new SqlConnection()");
}
catch(Exception ex)
{
     var usedTypes = context.UsedTypes;
     var missingTypes = context.MissingTypes;
}

This would be perfect to help me report back to my logs / UI what exactly went wrong.

...

Also I think I managed to fix the initial problem I was having with pure luck which was related to this so I can definitely see this saving time.

As always, Love your work man !!!
Thank you to you and your team for all the hard work on this!

[JonathanMagnan]

Thank you for your good word ;)

In fact, if we success to compile when the option context.RetryAndFindMissingTypes = true; is enabled, it will return the compiled version, however, the missingTypes will not be null.

var context = new EvalContext();
context.RetryAndFindMissingTypes = true;
var compiled = context.Compile<Func<object>>("new SqlConnection()");

if(context.MissingTypes != null)
{
	 var usedTypes = context.UsedTypes;
	 var missingTypes = context.MissingTypes;
}

[TehWardy]

oh I c ... enabling that function does a sort of "safe compile" so it'll succeed but produce null in the variable "compiled" in the event that types are missing.

Ok I can work with that :)
Thanks!

[JonathanMagnan]

Almost...

In actually return the compiled version (but doesn't cache it), so you can actually execute it. However, you know that the code was retried with all domain assemblies when the variable RetryAndFindMissingTypes is not null.

Perhaps we should have raised an error instead as you did with the try/catch!

[TehWardy]

Yeh so my way assumes that it tries, fails internally, sets up the collections exposed then throws a "Z.EvalException" or something.

That's a more "typical best practice" way of handling this kind of scenario (i think)

[JonathanMagnan]

Since the feature is pretty fresh and outside of you, none know about it, we will think about it tomorrow and maybe do it this way ;)

[TehWardy]

Sounds good :)
Awesome being able to feedback like this!

[TehWardy]

Some further thoughts i've had over night on this ...

I'm thinking the other up side here is that if anything at all goes wrong the context internally should catch that and attempt to provide some explanation / extended information.

In this situation where we know the failure is due to Missing types when I do this ...

var context = new EvalContext();
try
{
      var compiled = context.Compile<Func<object>>("var x = new SqlConnection()");
}
catch(EvalContextException ex)
{
     var reason = ex.Message;
     var missingTypes = ex.MissingTypes;
     var stack = ex.StackTrace; 
}

... So key changes I'm "suggesting" here ...

• The extra information is contained in a custom exception type so the context remains clean and free of "exceptional situation clutter".
• The message can be used to allow you to attempt to provide more detail about what was wrong.
• The stack might be more useful if it contained my code stack up to the problem line (not sure how hard this is to achieve though.

... so for example The stack trace for the above code might look something like this (ideally) ...

 at new SqlConnection() in Dynamic\Expression:line 1 
at Z.Expressions.CodeCompiler.CSharp.ExpressionParser.ParseSyntax(ExpressionScope scope, SyntaxNode node, Type resultType)
   at .[](EvalContext , String, IDictionary`2, Type, EvalCompilerParameterKind, Boolean, Boolean)
   at Z.Expressions.EvalContext.Compile[TDelegate](String code, String[] parameterNames)
   at Workflow.FlowInstance.BuildAssign[T](T activity, Flow flow) in D:\Core Prototype\Everything\Workflow\FlowInstance.cs:line 247

The first line here is a stack trace line from my string to be compiled.
Then signs the compiler did some compilation work.
Then my call to the compiler.

... This request about stack traces is probably a big ask and I have no idea how achievable it is as I don't know what level control / detials you have at the point an exception occurs.

My thinking is that "EvalContextException" could derive from something like AggregateException so would typically contain the exceptions we currently get as an inner exception but then extend it with whatever helpful hints you guys are able to throw in on top.

This would give a more "rounded and typical" .Net experience, and give the caller the option to gather more information by catching the exception without it being always there as a property on the EvelContext bloating the object definition on your end.

[JonathanMagnan]

Thank you TehWardy for this additional research.

I will think about the exception. We usually try to avoid custom exceptions in our library but I must admit that sometimes it could be the best way to handle this.

[TehWardy]

Fair enough ... it might be enough to throw an Aggregate exception, but I thought the use of a custom exception type would allow you to extend it with additional information ...

consider this (in your code)...

try
{
     return InternalCompile<T>(expression);
}
catch(Exception ex)
{
     throw new EvalException(this, ex);
}

This way the EvalException would contain the Context that was at fault, and you could "over time", as more tickets come in extend the EvalException class without having to make changes to the EvalContext.

In theory at least.

[JonathanMagnan]

Hello @TehWardy ,

Sorry for the late answer, I have been sick for a few days and was unable to review & release change made by my developer (had to catch up after 100+ email and still trying to catch up...)

The v2.9.51 has been released.

The option has been renamed to RetryAndThrowMissingTypes

and now throw an exception

var context = new EvalContext();
context.RetryAndThrowMissingTypes = true;
try
{
      var compiled = context.Compile<Func<object>>("var x = new SqlConnection()");
}
catch(Exceptionex)
{
     var reason = ex.Message;
     var missingTypes = context.MissingTypes;
     var stack = ex.StackTrace; 
}

RetryAndThrowMissingTypes

Turning on this option is required since retrying the code to find missing type have a performance cost so we cannot enable it by default

Custom Exception

We though a lot about your proposition on custom exception. Unfortunately, we choose to keep it this way. One major reason is that currently, almost every time this exception would have been thrown, no additional information would have been provided since only the missing types would have added some additional information.

There are some other reasons (honestly, I don't remember all what we said) but the final decision was to keep it simple and just throw a normal exception.

[TehWardy]

Hey @JonathanMagnan ,

Sorry to hear you were ill, hope you're back to 100% again soon, i know the feeling of coming back to a long list of emails lol.

...

Ok sounds reasonable.
I'm happy to go with this as it solves my problem.
Some thoughts for the future if you ever do come back round to this ....

Have the context run with the option turned off and in the event of a failure auto turn it on and run again to grab the extra details. This would negate the need for the user to set the extra flag which could then purely be an internal implementation detail for what I think is going to be a pretty standard need any time there's an input issue to the context.

From the description you implied that you weren't using a custom exception type but in the code sample you do ... can I assume the code sample is right as this looks pretty user friendly to me?

[JonathanMagnan]

Yup, going great now ;)

Oh damn, you are right about the example. I just edited it, it looks like I copied your text instead to create a real example. There is not custom exception, unfortunately.

Have the context run with the option turned off and in the event of a failure auto turn it on and run again to grab the extra details

If I understand correctly, this is what we want to avoid exactly. Image the following code

Eval.Execute("1+");

The expression is invalid, but if we automatically check for missing types, it will takes way longer to report the error. When looking for missing types, we register all domains types (and there is a lot!)

[TehWardy]

Yeh I completely agree:
My thinking was that internally behind the call I make the context would do something like ...

public class EvalException : AggregateException
{
       public Type[] MissingTypes { get; set; }

       internal EvalException(Exception innerException, EvalContext context)
       {
             InnerException = innerException;
             MissingTypes = context.MissingTypes;
       }
}

public class EvalContext
{
    internal bool RetryAndThrowMissingTypes;

    public T Execute<T>(string expression)
    {
        try
        {
            // hopefully this works first time so no additional time cost
            return ExecuteInternal<T>("1+");
        }
        catch (Exception ex)
        {
            // as the caller I foot this cost by default in orderfor you to obtain the fault info
            if (!RetryAndThrowMissingTypes)
            {
                RetryAndThrowMissingTypes = true;
                return Execute<T>("1+");
            }
            else
                // to be potentially implemented later ???
                throw new EvalException(ex, this);
        }
    }

    internal T ExecuteInternal<T>(string expression)
    {
        // your internal implementation here
    }
}

I'm sure there's tons more going on but the basic idea behind this is that I could then do ...

try
{
     var ctx = new EvalContext();
     var result = ctx.Execute("Bang!");
}
catch(EvalException ex)
{
     log.Error("Woops, this happened!", ex);
     log.Error("The context says that these types are missing:\n" + ex.MissingTypes.Select(t => t.Name).ToArray());
}

Context for my use ...

I have a web based UI that builds a json blob that represents a "piece" of a workflow.
I have code that translates that in to c# and hands it off to the context for compilation (and later execution).

The key complexity here is that I do this with many expressions at different points then build a new code tree that puts them all together and dynamically execute that.
In the event of a failure due to my design I know which exact expression went wrong but right now I don't know why.

The proposed fix will provide the answer to the most "common problem" but if I understand your explanation correctly I need to set this flag on the context "RetryAndThrowMissingTypes" which comes with a performance overhead to compute the extra information.

I think I can safely say that the key difference is that the when new option is turned on by the context "as needed" but is otherwise turned off that's the best performance point I can reasonably achieve.

The net result is that if this (as i'm proposing above / similar) isn't the implementation i'm always going to turn the option on by default as I don't actually know when it will be needed due to the dynamic nature of the code i'm sitting on top of these calls, meaning i'm always going to take that performance hit as I understand it.

Whilst your proposal does give me the required information it falls on me to know when to enable the option.
I have no issue with the context taking a bit of time to produce an exception in the event that I feed bad input but the standard "critical path" for me as a consumer should surely be that I assume the context will successfully execute what I ask it to "unless it tells me otherwise".

Am I making sense here?