Skip to content

Basic investigation of exception thrown during evaluate process.#321

Closed
n1l wants to merge 1 commit intoscriban:masterfrom
n1l:bug/evaluate-throws
Closed

Basic investigation of exception thrown during evaluate process.#321
n1l wants to merge 1 commit intoscriban:masterfrom
n1l:bug/evaluate-throws

Conversation

@n1l
Copy link
Copy Markdown

@n1l n1l commented Jan 26, 2021

It's not a completed code, it's more like a question.

I found a reason I think, for the latest loop step, Current node is set to null but the node is mandatory context for all internal array functions.
I've added a code to demonstrate that.

Do you have anything to suggest what else I can check?

@xoofx
Copy link
Copy Markdown
Member

xoofx commented Jan 26, 2021

I found a reason I think, for the latest loop step, Current node is set to null but the node is mandatory context for all internal array functions.

When/Where does it happen? I believe a proper fix would to avoid a dangling null node in the middle of eval, so if there is anything setting it to null, we should probably avoid setting null.

@n1l
Copy link
Copy Markdown
Author

n1l commented Jan 26, 2021

As I said it's not a completed code.

The exception throws when we are trying to get a result.
for ex:

var result = Template.Parse("{{['', '200', '','400'] | array.filter @string.strip}}").Evaluate(new TemplateContext());

var b = ((ICollection)result).Count; --> //bang!

Here:
image

I'm not sure what the correct behavior should be here. So I'm asking what else I can check.

@xoofx
Copy link
Copy Markdown
Member

xoofx commented Jan 26, 2021

I'm not sure what the correct behavior should be here. So I'm asking what else I can check.

That's what I suggested. Could you check when the node is going back to null? (e.g breakpoint/debug on the setter)

@NeilMacMullen
Copy link
Copy Markdown
Contributor

I think the issue is the same as explained in #322 - at least, the fix for that also allows the unit test in this PR to pass.

NeilMacMullen added a commit to NeilMacMullen/scriban that referenced this pull request Jan 27, 2021
@NeilMacMullen
Copy link
Copy Markdown
Contributor

Fixed by changes in #324

@n1l n1l closed this Jan 28, 2021
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.

3 participants