Which response type should wrapException have?

[sergioflores-j]

Current scenario

Currently we return a tuple of two positions:

[error: unknown, response: ReturnType<wrappedFunction> | undefined]

This has some limitations, as also described here:
#8 and #29

In short, TS is unable to determine if the response the consumer got was an error or a success in a easy/well typed way.
That is because unknown is the strongest type and removes the undefined definition.

const myWrappedFn = wrapException(async () => 'string');
const [error, response] = await myWrappedFn();

// error: unknown ; response: string | undefined
if (error) {
  // do error handling
  return;
}

// The type here is still:
// error: unknown ; response: string | undefined
// whereas it should be:
// error: undefined ; response: string

This has forced consumers to create utility functions like the one here: #8

Possible solutions

Tuple with extra parameter

You can use this Typescript playground to check: TS Playground tuple

The definition would be something like:

export type ErrorResponse<ErrorTypes = any> = readonly [
    error: ErrorTypes,
    response: undefined,
    isError: true,
];
export type SuccessResponse<Response = any> = readonly [
    error: undefined,
    response: Response,
    isError: false,
];

Consuming this would be something like:

const [error, response, isError] = await myWrappedFn();

// error: unknown ; response: string | undefined; isError: boolean

if (isError) {
  // error: unknown; response: undefined
  console.error(error);
  // do error handling
  return;
}

// The type here now is:
// error: undefined; response: string

Pros

• This is probably not a breaking change and consumers can still use the functions the way they did before with the addition of this new property.
• With the tuple the error is the most important part, and forces the consumer to think about it.

Cons

• It appears to be quite troublesome to set variables in files using multiple wrapped functions

const [error, response, isError] = await myFn1();

// code gets creative here
const [errorMyFn2, responseMyFn2, isErrorMyFn2] = await myFn2();
const [errorMyFn3, responseMyFn3, isErrorMyFn3] = await myFn3();

Object response

You can use this Typescript playground to check: TS Playground object response

The definition would be something like:

export type ErrorResponse<ErrorTypes = unknown> = {
  isError: true;
  error: ErrorTypes;
  /**
   * @warning check `isError` property before accessing `data` or `error`
   */
  data: unknown;
};

export type SuccessResponse<Response = unknown> = {
  isError: false;
  error: undefined;
  data: Response;
};

Consuming this would be something like:

const myFnResponse = await myWrappedFn();

// myFnResponse = { error: unknown ; data: unknown; isError: boolean }

if (myFnResponse.isError) {
  // myFnResponse = { error: unknown ; data: unknown; }
  console.error(myFnResponse.error);

  // do error handling
  return;
}

// The type here now is:
// myFnResponse = error: undefined; data: string

Pros

• With objects the code can grow indefinitely as obtaining the results for wrapped functions can be groupped in a single variable

const myFn1Response = await myFn1();

const myFn2Response = await myFn2();
const myFn3Response = await myFn3();

• We can enforce the consumer to check the isError by setting the data to unknown otherwise.

Cons

• It is a breaking change for the library
• Error attribute can be easily "ignored", as now the most important attribute becomes isError.
• It is slightly harder to destructure and rename the values from the response object.

To be defined:

Attribute names

isError
data
error

or

success
data
error

Which is best? Other options?

[MarcioMeier]

I'd rather to stick with the object approach because with the tuple approach makes hard to follow a flow when it comes with 2+ wrappers, as the names are likely to be very similar.

Also, Javascript engineers are quite used to work with objects so it shouldn't be hard to follow the new interface.

Regarding the attributes names, I'd go with isError, as this library mostly is about handling error, it makes sense to highlight it.

I'm just not sure about the isError because the result is not an error itself. Maybe hasError is more intuitive

[flaviostutz]

I like the object approach also and agree with the arguments that JS devs are used to handle this kind of returns whereas returning destructured array is not that common at all... it also makes things much better typed.

[sergioflores-j]

Then object it is!
Thanks for the input!

Implemented here #30
And will be available in v2.0.0-beta.1 for initial test rollout.