WIP: Throw exceptions when a programmer has done something wrong

[Nihlus]

This PR is part of a long-overdue comb-through of the library, wherein issues arising from programmer error (invalid API usage, not reading docs, etc) and not user input should raise exceptions instead of returning results.

I'm opening this as a draft PR in order to get community feedback on this more hardline approach, which is - at least theoretically - already in use in the library today. I've been relatively sloppy with this, and I need some extra eyes on this in order to get things reasonably right.

[Hamsterland]

I appreciate all the effort with this PR, but I disagree with these changes; either because I am misinformed as to what their benefits are, or because I strongly believe Result usage should be unilateral.

However, I am open-minded to this. What would you say is an example of "invalid API usage"? Regardless, if it is the developer who makes a usage mistake, then surely they'd rather be told that in a Result which they can handle gracefully, rather than an exception?

[VelvetToroyashi]

I think they're a welcome change and brings the library more in line with the well documented philosophy

[Nihlus]

The core idea is to fail fast when you, as the developer, have done something that cannot be handled gracefully - the best comparison I have is C++'s std::terminate() function or Rust's panic!() macro. The class of problems that should result in exceptions are those of blatant API misuse or bugs, not those that could arise from normal and expected use.

[VelvetToroyashi]

Furthermore programmer error is rather exceptional, and should be treated as such, whereas with APIs and commands, failure is only a possibility when something goes wrong, not a guarantee.

Most of these changes are exactly that, among with the areas I mentioned above.

[Nihlus]

Rust, for all the things I fault with its syntax choices, explains the difference really well here: https://doc.rust-lang.org/std/macro.panic.html#when-to-use-panic-vs-result

[Hamsterland]

Okay, I think I understand what you mean now. The rust panic!() macro put things into perspective. If I understand correctly, exceptions should only be thrown when the developer has grossly misused something in a way it was not intended to be used, and Results are returned from operational calls that have been used correctly. I agree with this philosophy, sorry I misunderstood.

[Nihlus]

Exactly right :slightly_smiling_face:

[AraHaan]

I disagree with these changes, if you want the developer to know they did something wrong that should be the job for an analyzer.

Examples:

• Microsoft.CodeAnalysis.NetAnalyzers
• StyleCop.Analyzers
• IDisposableAnalyzers
• etc

That way, when they did something wrong, it can be flagged as error by default to where their code would simply just fail to build, so failing to build would also fail to run.

[VelvetToroyashi]

• Analyzers aren't available in every environment
• Analyzers require a lot of time investment for something as intricate as a Discord bot, and would require constant updating with the nature of a library like this
• While analyzers are useful tools, they're not a substitute for A) People that know the library making certain assertions and B) having runtime failures that can be picked up in CI/Unit tests, whereas analyzers require a warning/error being emitted.

[VelvetToroyashi]

And given the already monumental workload Jax has to handle, I don't see an analyzer becoming an official solution in the foreseeable future, even if it weren't for the aforementioned caveats.

[AraHaan]

Actually since I know how to make analyzers, I think I could get started on one.

[VelvetToroyashi]

Furthermore An analyzer would be another dependency added that's constantly running over code just for very niche circumstances, further degrading its necessity. I don't think it's a good idea.

[Nihlus]

Having an analyzer would be a great complement, but not a replacement.