error handling strategy

[david-vanderson]

Error handling in dvui lacks a coherent strategy, but here's a proposal: "make it to the next frame".

Currently most dvui functions can return various errors, and there's no guidance on how to handle them. I personally have been putting "try" in front of everything and the whole app goes down if there is an error.

What could you do if a dvui function returns an error? We can imagine:

1. something is horribly wrong (like out of memory) and likely the app will crash, or at the least many things will start erroring
2. something temporary went wrong (like a backend dropped a texture)
3. something is consistently wrong with this part of the code (like misusing the dvui api)

I'm proposing moving to a strategy where if dvui encounters an error, it will log it and continue as best it can, skipping whatever needed to make it to the next frame.

This has the benefit of reducing the use of "try" all over application code, and shifting the burden of dvui error handling from the application to dvui. It also can turn temporary bugs from crashers into "this part didn't render", which is a clear win for robustness.

I'm thinking:

• keep errors on some setup functions like dvui.Window.init()
• one-shot functions can swallow errors like dvui.button()
• other things can return placeholder objects on error so the code can still progress but the functions are stubbed out

Any feedback about this would be helpful!

[rohlem]

It also can turn temporary bugs from crashers into "this part didn't render", which is a clear win for robustness.

My only concern with this strategy would be that a clear "oh, it crashed, and here is the error trace" might become "huh, this doesn't render, I don't know what's wrong with it/ the code".

I'm not sure what code structure you have in mind - would it be feasible/helpful if the package user could configure dvui's error handling strategy? It could be a single flag of crash vs swallow errors, or as fine-grained as feasibly useful.

EDIT: To keep the implementation simple/uniform even with toggle-able error strategy, we could introduce internal helper functions like so:

/// strips error sets from error unions if we swallow errors
fn DvuiSwallowedErrorResult(comptime Full: type) type {
  if (!swallow_errors) return Full;
  return @typeInfo(Full) {
    else => Full,
    .error_union => |u| u.payload,
  };
}
/// returns alternative_result if we swallow errors, otherwise returns error e
fn dvuiSwallowError(e: anytype, alternative_result: anytype) (if (swallow_errors) @TypeOf(alternative_result) else @TypeOf(e)) {
  if (swallow_errors) return alternative_result;
  return e;
}

// some functionality implementation
fn something() DvuiSwallowedErrorResult(SomethingError!SomethingResult) {
  const stub_result = SomethingResult.stub;
  // ...
  if (wrong) return dvuiSwallowError(error.Something, stub_result));
  // ...
}

A helper function like DvuiSwallowedErrorResult in the result type doesn't work with inferred error union types like !SomethingResult, however if the inferred error set is empty error{} every function just needs a short proxy function to hide / remove the empty error set. It's a bit annoying, but doable.

[david-vanderson]

My only concern with this strategy would be that a clear "oh, it crashed, and here is the error trace" might become "huh, this doesn't render, I don't know what's wrong with it/ the code".

I'm not sure what code structure you have in mind - would it be feasible/helpful if the package user could configure dvui's error handling strategy? It could be a single flag of crash vs swallow errors, or as fine-grained as feasibly useful.

Good thinking - this is a valid concern. I'm not sure exactly how this will work, but here are some thoughts.

I'm imagining in the place where dvui catches an error, it always logs to stderr, and then (optionally?) stores the error (or maybe a limited number of errors). The app can check every frame if there is a stored error, and either crash or do nothing, or maybe in the future show a user-friendly error-reporting thing?

I think dumping something to stderr is enough for developers during app development. We can outline in red the widget the error was in. If that's not enough we could have a switch to crash on all errors.

I'm not sure if overloading the return value fits well here - we want something where the code doesn't have to change between development and production.

It'll take a few tries to find something that works, but we've gotten good feedback on the duplicate id error handling (outline problem widgets in red, log to stderr, but keep going), so let's try going further in that direction.

Thanks for thinking about this!

[phatchman]

Almost all of the try's are allocation related. Most of the time the only reason install() is a try is because register() uses allocPrint, or something is added to the path ArrayList etc.

Potentially you could return a pre-alocated or static widget in these cases and log the error? Every failed allocation would then return the same widget.

I like that it would give error returns more meaning as the number of things returning errors would decrease to only those that likely need you to handle the error.

[david-vanderson]

We are definitely thinking along the same lines. If you look at LabelWidget.init() it has an attempt at this kind of this. In the allocation failure it logs it and then puts "<Error OutOfMemory>" as the label.

I don't know how this works for other kinds of allocations, we'll have to feel it out as we go.

[phatchman]

This is a very rough idea of what I was thinking.

It surprisingly does actually produce a "working" dvui app, but obviously it needs much more refinement.

There are downsides in that every widget will need to have an "OOM"/Mock constructor to put it into some reasonable state and it would be difficult to test these constructors without some supporting OOM testing infrastructure. And that could be a lot of effort to go to for a situation where the UI is likely going to crash anyway? But it does mean you don't have to think about how to deal with every failed memory allocation buried deep down in the call stack.

The benefit is that this pattern could be used for other things than OOM. Any failure we can't expect the user to reasonably do anything about could be handled this way.

This would make the dvui "creation" functions the boundary where only actual non-OOM errors are propagated. Not sure if that is a good thing or a bad thing?

[RedPhoenixQ]

I had another idea that is simpler but less flexible. If I remember correctly, most allocations causing the errors are on the arena. Perhaps we can assume the arena will never fail, as it retains its capacity anyway.

Assuming the arena can't fail allows for all drawing APIs to be infallible, which is most other errors I believe. Downside being that the user cannot control what happens if there is an error. I had some idea about calling a function like "dvui.catch" passing in the error and letting a comptime flag decide if the error gets logged and ignored or panics.

Additionally I think all the debugging options that might use allocPrint or similar should not contribute to error signature of the functions. Just crash/log/ignore if we cannot display debug info.

[phatchman]

When you say assume can never fail, what exactly do you mean? What would happen if it did actually fail? Getting rid of allocation errors in a simpler way, that would be ideal!

[RedPhoenixQ]

I mean doing something like "catch |err| dvui.catch(err)" instead of "try" in all internal allocations that use the arena. The functions could then just panic with the error or log it and we can figure out a way to exit early without an error.

[RedPhoenixQ]

Things that use the GPA like toast and dialogs might be worth to keep their errors exposed, as they are more rarely used. The data api I believe already doesn't surface errors. I don't think Window begin and end returning errors is an issue.

[phatchman]

I mean doing something like "catch |err| dvui.catch(err)" instead of "try" in all internal allocations that use the arena. The functions could then just panic with the error or log it and we can figure out a way to exit early without an error.

Gotcha. Yeah, I don't think the level of complexity I was proposing is worth it.

[david-vanderson]

I like the ideas here. One situation I can imagine is a grid or plot with user-controllable amounts of data. If the user makes a mistake and puts way too much stuff into it, ideally we do something better than panic. Best case is we can visually show that something went wrong. Second best is visually nothing and log an error.

[RedPhoenixQ]

True. Maybe an exception for widget with variable user controlled data? Or default limits for max points/visible rows?

There is also a limitation today where if a user provides a large amount of data, the area will retain that size until the end of the program. That might not be desirable. Perhaps arena should retain up to a certain amount of configurable capacity to combat that?

[RedPhoenixQ]

Just remembered that much of the backend vtable does have errors, so some backends that need to use the arena there just logs/panics today. Should probably get fixed/have these changes applied as well.

[david-vanderson]

There is also a limitation today where if a user provides a large amount of data, the area will retain that size until the end of the program. That might not be desirable. Perhaps arena should retain up to a certain amount of configurable capacity to combat that?

Good point, will need to address that. Hopefully we can figure out something that doesn't require a configuration.

I don't know how to handle backend errors, haven't thought about them much so far.

[RedPhoenixQ]

Tomorrow I might be able to try some stuff.

For the arena retaining memory, I'm thinking that we can retain the maximum amount used in the past X amount of frames, maybe with some extra capacity, so that it will be able to decrease over time.

[david-vanderson]

I think #374 fixes this. New issues can be opened for any followup work. Thank you!