Standardize and document the errors that can arise from builtins

[jmcook1186]

Spun out from #337

What
Standardize the errors that can arise in our builtins and document them on if.greensoftware.foundation

Why

To support our efforts

Context

To support our efforts to improve the IF debugging experience, we first enumerated the error types and messages that can arise from our core IF features. However, when we began doing the same for the plugins we maintain, we realized they were too disparate and non-uniform to document well. We interpret this as a signal that we need to standardize the way we surface errors from our standard library of builtins, including those that are being migrated in from if-plugins.

To this end, this ticket covers the standardization of the errors surfaced by our builtins into a finite set of types. The task is to go through all the error and log messages emitted by our builtins, drop/replace any that can be dropped/replaced so we maintain the minimum viable set of error types, and ensure all the error messages emitted across builtins are attached to one of those types. For example, no errors should be handled using console.log(<error message>), but instead should be handled similarly to throw new InputValidationError('Config is not provided.').

Once this is done, the remainder of the task is to document these error types and messages.
For each builtin, the README should include a comprehensive list of the error messages it can emit, organized by error type and including an explanation of the conditions that can cause the error to be thrown.

e.g.

---

Example: Groupby README

... rest of readme ...

Errors

InputValidationError

message	reason	remedy
'Config is not provided.'	The configuration data required by the groupby plugin was not available	make sure you are passing the correct config data to the groupby plugin, as explained [here](link to relevant docs)

---

Prerequisites/resources
Plugins need to have been migrated over to builtins (in progress)

Acceptance criteria

• error messages all use the ErrorType: error message syntax and use to a known, minimal set of types.
  • GIVEN the suggested changes have been merged
    WHEN I run ie --manifest <manifest> with errors
    THEN the error is always attached to a known error class
• the set of error types are defined in a single file that is available to all IF features and builtins
  • GIVEN the suggested changes are merged
    WHEN I navigate to if/src/util
    THEN I find a file errors.ts that contains an array CUSTOM_ERRORS that lists 100% of the defined error types supporting all the IF features and builtins
• the set of error types is documented on if.greensoftware.foundation
  • GIVEN the documentation exists
    WHEN I navigate to if.greensoftware.foundation/reference/errors
    THEN I see the list of error types with a brief explanation of the kinds of behaviours that cause them to be emitted, with real examples from our codebase
• the README for each builtin includes a comprehensive list of the error messages the builtin can emit, with reasons and remedies linking to the appropriate docs.
  • GIVEN the suggested changes have been merged
    WHEN I visit if.greensoftware.foundation/reference/errors
    THEN I find a comprehensive list of the error types F can emit and understand when and why they are used.
• unit tests are updated where necessary
  • GIVEN the suggested changes have been merged
    WHEN I run npx jest --coverage
    THEN I see a coverage report showing 100% coverage and 100% passing

SoW (scope of work)

• audit and update error types used across builtins and core features
• add README docs
• add docs to if-greensoftware.foundation

[jawache]

@jmcook1186 is this different from the list of exceptions 3rd party plugin authors will be able to raise? I suspect it's the same right.

Re Naming:
Could I request that when we gather the list of exceptions, names, descriptions etc... that we review the names and make them more obvious and clear. For the example above we are calling it an InputValidationError but from the description I would just call it a "MissingConfigError", we call other things "Inputs" so the name can be misleading if they didn't read the message properly. Also I'd just say what it is, give a clue to the end user as to what they have to do to fix the problem, reduce the cognitive burden as much as possible.

Re Packaging:
Right now the way we tell people to write code is to copy and paste all the types, errors, exceptions etc... into their own repos which seems an unprofessional way for us to ask 3rd party authors to engage with us. If it's the normal pattern with JavaScript projects then that's obviously fine (and it might be, not sure!) but if this was say a Java or C++ program we would create an if-core repo which is where we put things like common types, objects, files. We include that in if and 3rd party authors also include that in their projects to make sure we are all using the same code and names.

NOTE: If we go this approach it needs to work for both plain old javascript and TS, we shouldn't force everyone to use TS.

[jmcook1186]

Hi @jawache

This is just trying to get all the errors we currently emit under control and properly documented so we can have an informed conversation for #600.

The list of error types and messages is supposed to cover all the if features and builtins i.e. all the code we maintain, so that anyone can lookup, understand and fix the errors we emit.

For plugin builders the current assumption is that we will explain and encourage our practices but won't impose hard requirements on how plugins handle their errors. We don't want cumbersome error handling protocols to be a barrier to quickly executing prototype plugins, in my opinion.

However, this is something we need to discuss in detail in #600

[jawache]

Thanks @jmcook1186 ,

For plugin builders the current assumption is that we will explain and encourage our practices but won't impose hard requirements on how plugins handle their errors. We don't want cumbersome error handling protocols to be a barrier to quickly executing prototype plugins, in my opinion.

So I'd like to correct that assumption somewhat. The interface to plugins is both the function signature we expect but also the exceptions that they should throw. Both of these together form the "interface" to the plugin and the contract the plugin has with the If framework.

They can have whatever exception names they want for their internal logic - we're not forcing that at all, call internal logic whatever you want. But exceptions that they pass up to IF should be from a library of exceptions that we ask them to throw in different situations. MissingConfig being a great example of a very common problem. The truth is we're not going to do anything with that exception, we'll just log it and exit. But If every plugin author came up with their own name for an exception for when config is missing then this creates a lot more confusion in the manifest developer when they are diagnosing issues.

There are also a set of exceptions which we should explore where the plugin author can ask for some help, we've discussed before something like a RetryAfterException where the plugin throws that error to ask the framework to try again after a certain timeout (to get around rate limits in API calls). I'm not sure if we will implement it but it's just an example of a way the plugin is using exceptions to communicate back to IF, we might be able to handle errors more gracefully or even recover from errors.

So I agree the first step is getting it all documented, but the second step is figuring out what's missing and deciding if we want to supporting those as well (and also perhaps renaming them to be easier to understand)

[jmcook1186]

ok, thanks for explaining. We are on the same page - let's get the full set of current exceptions enumerated and then we audit and refine.

[jmcook1186]

Error classes that are used across builtins are documented here
Green-Software-Foundation/if-docs#71

[jmcook1186]

Closing as this is rolled into #593