API Design: Throw, log, or do nothing?

[tig]

With Dim.Auto (#3062) we'll have the ability for a dimension to be specified as "auto, but never smaller than 'min'": view.Width = Dim.Auto (min: 10).

It is possible for the super view's bounds to be less than 10.

What should we do when we detect this?

1. Throw an exception throw new InvalidOperationException ($"DimAuto specifies a min size ({autoMin}), but the SuperView's bounds are smaller ({superviewSize}).");.
2. Log a Debug spew message
3. Do nothing and let the app misbehave

I incline to "fail fast" with an exception.

[BDisp]

I incline to "3" because Dim.Auto depends of the superview size which when is smaller than the min size the subview will only be showing on the available space.

[tznind]

I think we want to avoid crashing over layout concerns where possible. If its a crash error to be in this state then user only finds out when resizing application really small, or running on a supet small terminal resolution (e.g. embedded device or something). I think that like css it should 'do its best' and document the fallback logics (i.e. min is not respected when parent bounds are less than)

…

On Tue, 13 Feb 2024, 18:53 Tig, ***@***.***> wrote: With Dim.Auto (#3062 <#3062>) we'll have the ability for a dimension to be specified as "auto, but never smaller than 'min'": view.Width = Dim.Auto (min: 10). It is possible for the super view's bounds to be less than 10. What should we do when we detect this? 1. Throw an exception throw new InvalidOperationException ($"DimAuto specifies a min size ({autoMin}), but the SuperView's bounds are smaller ({superviewSize}).");. 2. Log a Debug spew message 3. Do nothing and let the app misbehave I incline to "fail fast" with an exception. — Reply to this email directly, view it on GitHub <#3247>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHO3C5CVDMUIC4ITVP7YYF3YTOZDZAVCNFSM6AAAAABDHASFLWVHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZWGIYTIOBQGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[dodexahedron]

It's entirely situational, honestly.

But the guidelines for handling exceptions are relatively deterministic.

In general, if an exception is thrown and:

• If it is benign and simply notifying the user is sufficient, then:
  • Alert if appropriate and log if possible, at low severity
• If it affects program state and
  • If it is recoverable, then do all of:
    • Revert any work done in that method, up to that point, to leave things as they were before it was invoked
    • Either return an appropriate result, if the method does that or, if not, re-throw
    • Log at least at warning or info
    • Alert the user, if appropriate
  • If it is non-recoverable and:
    • If the exception handler cannot guarantee consistent program state, then:
      • Depending on just how fatal or dangerous the exception was, attempt to log at fatal severity. If it's bad enough, don't even attempt it.
      • Terminate immediately, with Environment.FailFast()
        • This should be rare in a library, and should only happen if continued execution could lead to data corruption or security issues like executing unknown code or processing unknown values, especially in native interop code
    • If the exception handler can be reasonably sure the program is fine, but just this operation failed in an unrecoverable way, then do all of:
      • Log at warn or error severity
      • Alert the user if appropriate
      • Re-throw and ensure callers within the library also handle the re-thrown exception by the above rules (which may be a different evaluation at the caller's scope)

[dodexahedron]

In addition to that, for handling of exceptions, the guidelines for throwing them and for re-throwing them are worth mentioning, too, much of it just copied directly from the Best Practices doc because I got lazy and didn't paraphrase as much:

• Handle common conditions without throwing exceptions
  • Use exception handling if the event doesn't occur often, that is, if the event is truly exceptional and indicates an error, such as an unexpected end-of-file. When you use exception handling, less code is executed in normal conditions.
  • Check for error conditions in code if the event happens routinely and could be considered part of normal execution. When you check for common error conditions, less code is executed because you avoid exceptions.
• Design classes so that exceptions can be avoided
• Throw exceptions instead of returning an error code
• Use the predefined .NET exception types.
  Introduce a new exception class only when a predefined one doesn't apply. For example:
  • If a property set or method call isn't appropriate given the object's current state, throw an InvalidOperationException exception.
  • If invalid parameters are passed, throw an ArgumentException exception or one of the predefined classes that derive from ArgumentException.
• End exception class names with the word Exception
• Use at least the three common constructors when creating your own exception classes: the parameterless constructor, a constructor that takes a string message, and a constructor that takes a string message and an inner exception.
  • Exception(), which uses default values.
  • Exception(String), which accepts a string message.
  • Exception(String, Exception), which accepts a string message and an inner exception.

And plenty more.

There is a TON of good information, guidelines, suggestions, samples, and conventions in the entire Exceptions in the .net Fundamentals docs at Microsoft Learn.

[dodexahedron]

Specifically, for the case you presented in the OP, that seems like an appropriate place to throw an InvalidOperationException or something derived from it, if you want to be able to convey more detail to the consumer. Best to put the actual data points in the exception itself and then make the message a localizable string that can substitute those values as appropriate, in the ideal case.

As for how that behaves:

That's up to the consumer, just like any exception thrown by any dependency of their code. So long as we handle and re-throw appropriately up the stack, we're performing our due diligence.

We should try to avoid doing things that obfuscate details like that, because it restricts what the user can do and their visibility into how their program is executing.

[dodexahedron]

I'd also argue that, any time we throw our own exceptions as the first chance exception, we should include a logging hook at that point, as well.