Fix stack exception name

[superboyiii]

Close https://github.com/neo-project/neo-modules/issues/854

[Jim8y]

a new field?

[superboyiii]

a new field?

Two kind of exception(one from vm, one from applicationlog), but they're using the same trigger. applicationlog exception could rewrite vm exception...

[superboyiii]

Now it's the same result as invocation. @roman-khimov

[roman-khimov]

The behavior we have in NeoGo now is that we return an array of elements in the stack. They can be proper stack items or they can be strings. Suppose you have a script leaving 1 and some bad item on the stack, with NeoGo you'd get

"stack": [
    {"type":"Integer","value":"1"},
    "error of some kind"
]

With this PR that would be

"stack": "error of some kind"

And this obviously loses more data that can be valuable for debugging, the number of elements left on the stack and all the proper elements can be helpful in many cases. I'd prefer having this kind of output in the end both for application log and test invocations.

[superboyiii]

"stack": "error of some kind"

So if 1000 stacks return 1000 error messages? And they are the same hard code message...

[roman-khimov]

One stack item --- one error message, you serialize them one by one anyway, either you get a proper JSON or you get an error.

[superboyiii]

@shargon What's your opinion?

[shargon]

If we add the stack, i prefer to add the stack item type, and the value, otherwise I prefer to use the exception field

[superboyiii]

Tested. @shargon @roman-khimov

[shargon]

Why we don't use the exception field?

[superboyiii]

Why we don't use the exception field?

These two exceptions are generated from different place, but applicationlog exception could re-write vm exception which doesn't make sense and can make some Dapp backend confusing a lot for handling these exceptions. For example: Ghostmarket.

[shargon]

Why we don't use the exception field?

These two exceptions are generated from different place, but applicationlog exception could re-write vm exception which doesn't make sense and can make some Dapp backend confusing a lot for handling these exceptions. For example: Ghostmarket.

What about change the exception field to be an array, also with type?

[roman-khimov]

If it'd be separated, you wouldn't know which elements are bad. In some cases it might be useful. Imagine 3 items on the stack, the second one is bad, but the other two are OK and can be serialized, if you get two stack items and one exception, what was the original index of the element that has caused this exception?

[cschuchardt88]

807 And #842 implements AppclaitionEngine.Log for the meantime in application log for debug mode. Until core can be update to do it properly.

[superboyiii]

Done. @roman-khimov @shargon

[superboyiii]

@AnnaShaleva Could you review again? Any more improvement needed?

[superboyiii]

@Liaojinghui @shargon Is it OK for you?

[superboyiii]

Done @shargon

[shargon]

Maybe add the error description will not be deterministic between neo-go and c#?

[cschuchardt88]

I think we should be doing this the proper way, indicating that there is an valid stackitem. We shouldnt be hiding it or saying there is something wrong with it; when there is not.

Edit:

807 and neo-express handle the stackitems this way.

[roman-khimov]

error description will not be deterministic between neo-go and c#?

Yes, it will differ.

[shargon]

error description will not be deterministic between neo-go and c#?

Yes, it will differ.

Then is better to change the message

[roman-khimov]

I think we can settle on the fact that if you're receiving a string instead of a stack item, it's an error string and the original stack item couldn't be represented. And this string can be implementation-specific, just like data in JSON-RPC errors.

[superboyiii]

@shargon @roman-khimov need approve