Improve Error Handling

[markbates]

Is your feature request related to a problem? Please describe.

Yes. Currently, in both the v2 and v3 versions of the API there is little to no error handling. The result is either panics or calls to log.Fatal. It's wilding frustrating to have a 3rd party library crashing my application.

In idiomatic Go we return an error if there is a problem. In the Wails API however, there are very little errors returned. Instead, when there is a problem the application either crashes with a panic or a call to log.Fatal. This makes applications nearly impossible to test and require the end user to wrap significant portions of the API to prevent these situations.

Describe the solution you'd like

I suggest idiomatic go error handling be implemented and nil checks are implemented.

Consider the following code snippet from the v3 API.

func (a *App) Capabilities() capabilities.Capabilities {
	return a.capabilities
}

If a is nil, then this code will panic and crash my applications and tests.

Consider the same code, but with a nil check and error handling.

func (a *App) Capabilities() ( capabilities.Capabilities, error ) {
	if a == nil {
		return capabilities.Capabilities{}, fmt.Errorf("app is nil")
	}
	return a.capabilities
}

Now, my application, or tests, will no longer panic and crash, but will return an error I can check and handle.

Describe alternatives you've considered

As an alternative to this I've had to wrap the entire v2 API in safe, idiomatic code, https://github.com/markbates/wailsx. This is, obviously, not a long term solution, especially with v3 in alpha. I do hope, however, that it might prove to be a guideline for further API discussion.

Additional context

No response

[leaanthony]

Hi Mark 👋

Nobody has previously commented on this but it's a valid point. Are you suggesting that we wrap every method with a nil check and return an error? Apart from a nil check, do you have any other examples?

Thanks! 🙏

[markbates]

Checking for nil is really big start. Prevent crashes is always a big win.

Here's another example from the v2 API where an error would be very useful.

func WindowSetSize(ctx context.Context, width int, height int) {
	appFrontend := getFrontend(ctx)
	appFrontend.WindowSetSize(width, height)
}

I could call this function with illegal values: WindowSetSize(cox, -1, -1). In theory, I shouldn't be able to set the size of the window to negative numbers. Returning an error from this function would catch this issue.

func WindowSetSize(ctx context.Context, width int, height int) error {
	if width < 0 || height < 0 {
		return fmt.Errorf("w or h is less than 0: %d, %d", width, height)
	}

	appFrontend := getFrontend(ctx)
	appFrontend.WindowSetSize(width, height)
	return nil
}

Does that help?

[leaanthony]

It does, and thanks for bringing the suggestion now as we can absolutely incorporate that into v3.

Just one point of clarity though, what are you using context for?

Cheers 👍

[markbates]

You tell me. :) Joking asides, that example is in the v2 API. Almost all of those functions take a context as the first argument.

[leaanthony]

Oh, THAT context. Yeah, that was simply a bad decision 😅 If you discord, it would be great to have you on the server for feedback on v3. If not, then this can work. Thanks again for raising it Mark.

[stffabi]

Yeah we absolutely need improvements on the error handling side.

Personally I'm a little surprised about the recommendation for adding all those nil checks of struct instance pointers. As you mention @markbates this is idiomatic Go, so I'm very interested in having some insights why that is seen as idiomatic. Are there any official references about that? IIRC I haven't seen that pattern often, also not that often in Go's own source.

[leaanthony]

Also, the door is very open to you to come help shape v3 😉

[iamkhalidbashir]

Also think error handing types is not correct for example for:

func (a *AppApi) GetMachineID() (string, string) {
	id, err := machineid.ID()
	if err != nil {
		return "", err.Error()
	}
	return id, ""
}

the wailjs runtime TS is

export function GetMachineID():Promise<string|string>;

Which is not the same, as TS expects to string either or, it can't diff between result and error message

[BigBallard]

My approach on this is simply interpreting pointer types as possibly being null or undefined somehow as this is the best translation to intention.

For example:

func GetOrder(id string) (Order, *AppError) {
  ...
}

could translate to the TS signature:

export function GetOrder(arg1: String): Promise<Order, AppError|undefined>

Now I could see this becoming messy with multiple return types that could all be pointers, so another approach would be to create a wails generated result type that can have each return type be a field of the result.

Example:

/* 
export interface <FUNC_NAME>Result {
  <TYPE_LC>: <TYPE[|undefined]>
}
*/
export interface GetOrderResult {
  order: Order
  error: Error|undefined
}
export function GetOrder(arg1:String): Promise<GetOrderResult>

This approach would prevent the need panic to force an error on the frontend while making less overhead on the Go side.

Thoughts?