Ability to call with error test.

[reduz]

Adds a new function Object.call_with_error_test

Allows to tell if the call of a function fails and the reasons behind the failure. Should help better implement unit tets and other situations.

Usage example:

var err = CallErrorInfo.new()
var ret = object.call_with_error_test(err,"function",args)

if (err.get_call_error() != CallErrorInfo.CALL_OK):
      print("Something went wrong")

Alternatively, to avoid strings:

var err = CallErrorInfo.new()
var ret = object.function.call_with_error(err,args)

if (err.get_call_error() != CallErrorInfo.CALL_OK):
      print("Something went wrong")

[jcostello]

Why the err variable?

var err = CallErrorInfo.new()

Seems to be redundant to be declared before the call. Can it be declared inside the call_with_error_test and return in ret?

[AThousandShips]

Would you suggest returning an array, or how would you return both the value and the error? This would also make the call harder to adapt to existing code by having you extract the return

[jcostello]

I was suggesting returning an object with error and value or ret. Extracting the value is not big of a deal. To me its tedious if I have to declare a variable before using it

var response = object.call_with_error_test(err,"function",args)

if (response.get_call_error() != CallErrorInfo.CALL_OK):
      print("Something went wrong")

You wont event need to ask error directly. But you can still have it if you need it from response.error

[AThousandShips]

That's more clear, thank you, but I think it still makes the code more convoluted

[reduz]

@jcostello The call can return a value if the function returns a value, so the return is needed.

[Bromeon]

Will error messages emitted by the failing call be silenced?
If errors are expected, it seems like that would be desired in most cases... 🤔

[jcostello]

@jcostello The call can return a value if the function returns a value, so the return is needed.

I was implying to have the error declared inside the call_with_error_test method and return as part as the response. I dont see the point of pasing it as a reference. To me it too C style of doing things

[adamscott]

@jcostello The call can return a value if the function returns a value, so the return is needed.

I was implying to have the error declared inside the call_with_error_test method and return as part as the response. I dont see the point of pasing it as a reference. To me it too C style of doing things

I concur, it makes it really C-like. Is it the first API where we pass an error?

[vnen]

I'm not sure if the approach here is ideal, I see a few flaws with it. Consider the following example:

@tool
extends EditorScript

func _run():
	print("beg run")
	var err := CallErrorInfo.new()
	call_with_error_test(err, "a")
	prints("err", err.get_call_error())
	print("end run")

func a():
	print("beg a")
	b()
	print("end a")

func b():
	print("beg b")
	c()
	print("end b")

func c():
	print("beg c")
	var x = self
	x.nope()
	print("end c")

The first issue I see is that this breaks compatibility in terms of how errors are treated and propagated. In current stable (4.3) the call to c() will fail because it tries to call a non-existent function. But it won't raise an error so b() will finish executing (and so will a() and _run()). With this PR, the call to c() will also give an error and it will be propagated until the first call to _run(). The test here will avoid _run() raising an error too.

If we remove this change in behavior, then the test call in this example will not be useful, because a() will not raise an error even though c() did. The call_with_error_test() will say the call went fine, so you cannot really "catch" the error deep down.

If we try call_with_error() to call c() directly, we will then get a SCRIPT_ERROR because that's what c() will raise. But this tells nothing about the actual error that happened within c() which is an INVALID_METHOD. This bubbling up of error does not preserve the original message, which reduces a lot the usefulness of this system. It will pretty much always get a SCRIPT_ERROR, so the CallErrorInfo object will almost never contain any useful information (might as well just be a bool instead of an object).

The other problem is the interface as some already pointed out. Passing an object is common in C/C++ but not in GDScript. I would suggest just returning an array with the first element being the value and the second being the error result (which may be just an OK). It would be more convenient with destructuring, but it's still better than having the create an object beforehand IMO.

For this to work as intended and be useful I would say it requires these points:

• The error should be passed no matter the depth of the call stack (not entirely needed but I think it would be more useful).
• The original error should be passed around, not replaced by another one.
• It should not change the execution behavior and allow functions that didn't error to keep running and not return erros. I know this creates the possibility of having more than one error to bubble up, but we can agree on a convention (like only passing around the first error found).

This is almost like exceptions in a way, but without the overhead of actual exceptions. Also, I would consider making this a GDScript construct instead of being an Object/Callable method. This way we could even consider having a different call flow when this is used. Would also be easier to validate the call at compile time.

[dalexeev]

The first issue I see is that this breaks compatibility in terms of how errors are treated and propagated. In current stable (4.3) the call to c() will fail because it tries to call a non-existent function. But it won't raise an error so b() will finish executing (and so will a() and _run()). With this PR, the call to c() will also give an error and it will be propagated until the first call to _run(). The test here will avoid _run() raising an error too.

At some point between 4.0 and 4.1 we broke (or fixed) this.