Errors in some cases are unusable/PR was reverted

[phxnsharp]

In #204, which was approved and merged with master I fixed and added test cases for several areas where error reporting was quite non-useful. Shortly after in 28d6aa2#diff-3e3487db6f5281f14631a689d9e902314cecb42d32976a21758dbbece8e7a801
those changes and the tests that prove they work were reverted or lost. I don't know if it was intentional or not.

However, the changes are actually very important for us as we would like good errors with line numbers to be reported in all cases for our customers. Can I ask for this to be revisited please?

[ClearScriptLib]

Hi @phxnsharp,

As we indicated in the PR comment here, we ended up adjusting the error handling code slightly and moving the tests to a different file after merging. At the time there were massive code changes taking place, as ClearScript was going cross-platform, so it's certainly possible that we broke something.

However, we just ran your original tests from the PR, and all five worked with the following text changes:

• Unknown documents show up as "[unknown]" rather than "Unknown" in the error messages.
• In VBScriptEngine_ExceptionDetails2, the error message is "Object doesn't support this property or method: 'test'" rather than "Class doesn't support Automation: 'test'".

Please let us know if you're aware of any additional changes or regressions.

Thank you!

[phxnsharp]

We have a suite of unit tests that test our integration with ClearScript. When we recently switched it from our custom compiled version from my fork to the latest release, we started getting a pile of failed tests due to error messages being much simplified. Based on that and seeing the mass deletions, I assumed it was totally broken. But I see that all 14 tests are really parameterized versions of a single test, and that other tests we put into place are still working. So it looks like just one case is broken.

The case is when you have provided a COM object to the script host and the script attempts to call a non-existent property. The expected error in our tests looks like:

    Object doesn't support this property or method: 'out1.description'
      at run (Script:3:18) -> out1.description = "new Value"

but all we are getting is:

   Invalid object or property access

There are a lot of permutations and I don't have my original PR up and compiling right now. The test I originally wrote named VBScriptEngine_ExceptionDetails_NoDebugger on line 545 of VBScriptEngineTest.cs in my PR might prove this case. Or, we might need to write a new test doing the same thing with ComVisibleTestObject (it might be specific to having a COM object).

Thanks for your help! Glad to know it is isolated to a specific case.

[ClearScriptLib]

Hi @phxnsharp,

The test I originally wrote named VBScriptEngine_ExceptionDetails_NoDebugger on line 545 of VBScriptEngineTest.cs in my PR might prove this case.

We've run all five tests from your PR on the latest ClearScript (currently version 7.1.5), and they all succeeded with the text changes above. One of these tests, VBScriptEngine_ExceptionDetails2, uses a ComVisible object.

Please let us know if you can isolate an errant case.

Thanks!

[phxnsharp]

I have someone trying to do that, but it looks like in the refactor the test projects were all removed from the "NoV8" solution. Is there still a way to run the tests without setting up V8?

[ClearScriptLib]

I have someone trying to do that, but it looks like in the refactor the test projects were all removed from the "NoV8" solution.

The ClearScript.NoV8 solution never included test projects; that's mentioned in the build instructions.

Is there still a way to run the tests without setting up V8?

Sadly, it wouldn't be trivial. You could add one of the existing test projects to ClearScript.NoV8, but then you'd have to strip out any V8-dependent test code.

Another option might be to create a basic console app, bring in ClearScript via NuGet, and add your test code. We'll be happy to investigate any issue you find that way.

Cheers!

[briell99]

Here is a test and helper class that isolates our issue.

        [ComVisible(true)]
        public class readOnlyTestObject
        {
            public string Description { get; }
            public readOnlyTestObject()
            {
                this.Description = "";
            }
        }

        [TestMethod, TestCategory("VBScriptEngine")]
        public void VBScriptEngine_VerifyDescriptionReadOnly()
        {
            engine.Dispose();
            engine = new VBScriptEngine(WindowsScriptEngineFlags.EnableDebugging | WindowsScriptEngineFlags.MarshalArraysByValue);
            string BodyText;
            BodyText = $@"
               sub run
                  out1.description = ""new Value""
               end sub";

            readOnlyTestObject test = new readOnlyTestObject();
            engine.AddHostObject("out1", HostItemFlags.DirectAccess, test);     
            engine.Execute("\n" + BodyText);

            try
            {
                engine.Invoke("run");
            }
            catch (ScriptEngineException see)
            {
                Assert.AreEqual(
                   "Invalid object or property access: 'out1.description'\n    at run (Script:3:18) -> out1.description = \"new Value\"",
                   see.ErrorDetails, "Details message was wrong");
            }
            catch (Exception ex)
            {
                Assert.Fail("Wrong exception thrown: " + ex);
            }
        }

[ClearScriptLib]

Hi @briell99,

Thanks for posting the test case! We've confirmed that it currently causes ClearScript to lose a portion of the error details. We'll have a fix in the next ClearScript release.

Thanks again!

[ClearScriptLib]

Fixed in Version 7.1.6.