Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get rid of 'reference not set to an instance of an object' error, which appears when context is null #1917

Conversation

Thrasha
Copy link
Contributor

@Thrasha Thrasha commented Jul 15, 2024

getting weird error when execute ts script in jint:
Method CalculateLoanConditions has thrown an exception. System.NullReferenceException: Object reference not set to an instance of an object. at Jint.Runtime.Interpreter.Expressions.JintExpression.GetValue(EvaluationContext context) at Jint.EsprimaExtensions.TryGetKey[T](T expression, Engine engine, Boolean resolveComputed) at Jint.Runtime.Interpreter.Expressions.JintObjectExpression.BuildObjectNormal(EvaluationContext context) at Jint.Runtime.Interpreter.Expressions.JintExpression.GetValue(EvaluationContext context) at Jint.Runtime.Interpreter.Expressions.JintCallExpression.ArgumentListEvaluation(EvaluationContext context) at Jint.Runtime.Interpreter.Expressions.JintCallExpression.EvaluateInternal(EvaluationContext context) at Jint.Runtime.Interpreter.Expressions.JintExpression.GetValue(EvaluationContext context) at Jint.Runtime.Interpreter.Statements.JintReturnStatement.ExecuteInternal(EvaluationContext context) at Jint.Native.Function.ScriptFunction.Call(JsValue thisObject, JsValue[] arguments) at Jint.Runtime.Interpreter.Expressions.JintCallExpression.EvaluateInternal(EvaluationContext context) at Jint.Runtime.Interpreter.Expressions.JintExpression.GetValue(EvaluationContext context) at Jint.Runtime.Interpreter.Statements.JintReturnStatement.ExecuteInternal(EvaluationContext context) at Jint.Native.Function.ScriptFunction.Call(JsValue thisObject, JsValue[] arguments)

https://stackoverflow.com/questions/78326684/error-object-reference-not-set-to-an-instance-of-an-object-after-upgrade-jint

after debugging jint, I've found out that for my scripts context is null. I substitute null to new EvaluationContext() and it started to work well

@lahma
Copy link
Collaborator

lahma commented Jul 15, 2024

Can you add a test case showing the original problem?

@Thrasha
Copy link
Contributor Author

Thrasha commented Jul 15, 2024

I will try to reproduce it wia unit test, however, I'm not positive with that. The script, generated by state machine is huge and complex, and I couldn't determine which part of script has context null when I was debugging jint part. Give me a day, I'll try to find something

@Thrasha
Copy link
Contributor Author

Thrasha commented Jul 16, 2024

Hi @lahma. Unfortunately, it is impossible to write a unit test due to protection level of _activeEvaluationContext. What else can I do to make this fix merged?

@lahma
Copy link
Collaborator

lahma commented Jul 16, 2024

Jint.Tests projects should be able to see internals?

…bject reference not set to an instance of an object'
@Thrasha
Copy link
Contributor Author

Thrasha commented Jul 16, 2024

It does see, however, we have to change private method TryGetComputedPropertyKey to internal. If yes, this following test throws JS error after my fix and 'Object reference not set to an instance of an object' before my fix

Copy link
Collaborator

@lahma lahma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating a test case, Added some improvements ideas.

Jint.Tests/Runtime/EvaluationContextTests.cs Outdated Show resolved Hide resolved
Jint/AstExtensions.cs Outdated Show resolved Hide resolved
Jint/Engine.cs Outdated Show resolved Hide resolved
Jint/Native/Function/EvalFunction.cs Outdated Show resolved Hide resolved
Jint/Runtime/Interpreter/Expressions/JintExpression.cs Outdated Show resolved Hide resolved
Copy link
Collaborator

@lahma lahma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did minor adjustments, thank you for the PR.

@lahma lahma merged commit 1b7288d into sebastienros:main Jul 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants