TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Talk about Exceptions Here

Open RyanCavanaugh opened this issue 2 years ago • 106 comments

Acknowledgement

  • [X] I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.

Comment

#13219 is locked so that the conclusion doesn't get lost in the discussion, so talk about exceptions here instead

RyanCavanaugh avatar Nov 10 '23 19:11 RyanCavanaugh

I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.

I love the irony that a maintainer was forced to check this box 😄

fatcerberus avatar Nov 13 '23 17:11 fatcerberus

https://github.com/microsoft/TypeScript/issues/13219#issuecomment-1806332624

@KashubaK @kvenn I'm going to start looking at intellij (and also see if I can make an eslint plugin for it!). If you want to help, let me know!

michaelangeloio avatar Nov 27 '23 14:11 michaelangeloio

Would be happy to be involved. I did like the idea of ESLint plugin because it's piggybacking off of an already established static analysis solution, but I think IDE plugins also check that box.

This comment has some code for the starting point of the ESLint plugin (in a collapsed text block): https://github.com/microsoft/TypeScript/issues/13219#issuecomment-1693733605

kvenn avatar Nov 27 '23 18:11 kvenn

Would be happy to be involved. I did like the idea of ESLint plugin because it's piggybacking off of an already established static analysis solution, but I think IDE plugins also check that box.

This comment has some code for the starting point of the ESLint plugin (in a collapsed text block): #13219 (comment)

@kvenn see my comment here about eslint https://github.com/michaelangeloio/does-it-throw/issues/70#issuecomment-1879726088

I've got jetbrains-intellij working now, but waiting on jetbrains to approve it! You can check the code for that here if you'd like: https://github.com/michaelangeloio/does-it-throw/tree/main/jetbrains

michaelangeloio avatar Jan 06 '24 20:01 michaelangeloio

Heck yes! I'll happily use the IntelliJ plugin. I'll check back in a bit and install when it's approved.

Shame that eslint doesn't support async and that it's a ways away. But there's even more you can do with a plugin.

Nicely done!

kvenn avatar Jan 07 '24 00:01 kvenn

@kvenn jetbrains is now available! https://plugins.jetbrains.com/plugin/23434-does-it-throw-

Feel free to share with others!

arivera-xealth avatar Jan 24 '24 21:01 arivera-xealth

I've since gotten the opportunity to try out the JetBrains plugin for does-it-throw and after looking into it a bit more, I don't think that really solves the problem I'm having with exceptions.

That plugin seems to mostly be about alerting of where throw statements are used. Which appears to be for enforcing that you don't use throw statements. I think throw statements are here to stay, even if I agree first class support for errors has advantages. And that if you're already in a codebase which relies on throws, this adds a lot of noise.

I had proposed an ESLint exception to warn when invoking a function that can throw. Encouraging you to either mark(document) this function as one that re-throws or to catch it. With the intention being to prevent you from accidentally having a function that throws bubble up all the way to the top of your program. But allowing that to be the case if it makes sense (like in a GraphQL resolver, where the only way to notify Apollo of the error is via throwing, or a cloud function / queue where throwing is used to retry).

If it can be found implicitly (without documentation), that's better. And it seems like a plugin could actually achieve that (and even offer quick fixes, which would be SO COOL). I'd advocate for using an already used standard TSDoc annotation (@throws) as the acknowledgement that this throw statement is there on purpose (as opposed to introducing a new one - @it-throws or @does-it-throw-ignore).

does-it-throw has some great bones. And it seems like it's solving a problem for others, it just might not be the right fit for me.

kvenn avatar Jan 29 '24 20:01 kvenn

@kvenn I've tried my hand at an eslint plugin: https://github.com/Kashuab/eslint-plugin-checked-exceptions/tree/main

It introduces two rules:

  • uncaught-errors

Checks to see if a function you're calling has a @throws JSDoc annotation. If you don't wrap the function call in a try/catch it will output an error.

  • undocumented-errors

Warns you if a function has a throw statement without a corresponding @throws annotation. Matches based on what you're throwing, in case you have custom error classes.

Check out the tests for examples and what it covers. It's been a while since I looked at this, but I remember it being a bit buggy (i.e. nested branches, complicated logic) so there's a ton of room for improvement. I might take another look to improve it.

Side note - the README suggests you can install it from NPM, this is not the case haha.

(This is my work GH account, dunno why I have a separate one but oh well. I previously contributed here as @KashubaK)

Kashuab avatar Jan 29 '24 21:01 Kashuab

I'm definitely a complete noob when it comes to how Javascript/Typescript works, but would it be possible to add the modifier throws to a Typescript function signature?

https://docs.oracle.com/javase/tutorial/essential/exceptions/declaring.html

As both a Typescript user and a library maintainer, I hate not being able to consume / deliver proper error declarations. I know I can use JSDoc's @throws, but having that keyword as part of the function signature would be so great...

wiredmatt avatar Mar 02 '24 01:03 wiredmatt

@wiredmatt That suggestion was discussed in detail in the linked issue: #13219

The TL;DR is essentially, it's not worth doing because there isn't sufficient existing practice/documentation/runtime behavior to facilitate such a feature. TypeScript is designed to fit within the scope of JS' behavior, and since JavaScript doesn't give us reliable, native tools for things like checked exceptions it's challenging to fit it within scope.

What we're pondering now is, what's the next best thing? How can we encourage better error handling practices enough that the community has some common ground to operate on?

KashubaK avatar Mar 02 '24 02:03 KashubaK

@KashubaK thank you for your response.

I was thinking about the newly introduced type guards / type predicates, in my mind it seemed totally possible, especially knowing we have conditional types as well.

I'll keep an eye on the eslint solution, that makes sense to me knowing what you just explained. thanks!

wiredmatt avatar Mar 02 '24 04:03 wiredmatt

I was thinking about actual "throws" keyword as optional in return type, so TS would automatically add defaults to current functions/methods .. something like throws<any> or throws<unknown> at least for starting point. so when we do write modules we can actually more strictly expose what type of things we are throwing out like example.

function doAuth(): AuthPayload throws<TypeError | AuthError> {}

and maybe have some utility type similar as typeof to extract errors types from function so we can more easily utilize other modules throw types without directly importing those.

function someStuff(): AuthPayload throws<throwof doAuth | FatalError> {}

Also maybe this have later impact on catch argument type to actually know throw types, but just actual documentation of throw types is way more important atm for interoperability between modules as currently we are just quessing and reading module source code to undestand what might actually get thrown.

Edit: this would also work for indication that function will never throw .. throws<never>

mharj avatar Mar 04 '24 05:03 mharj

Also maybe this have later impact on catch argument type to actually know throw types

This doesn't really work unless you have a level of information that doesn't exist in the real world, and requires the ability to express many patterns that are basically arbitrarily complicated. See https://github.com/microsoft/TypeScript/issues/13219#issuecomment-1515037604

RyanCavanaugh avatar Mar 04 '24 16:03 RyanCavanaugh

but just actual documentation of throw types is way more important atm for interoperability between modules

If documentation is the issue, you don't need a TS keyword -- https://jsdoc.app/tags-throws has existed for ages. I don't know about you, but I really don't see it used very often. This is the heart of the problem Ryan described in the comment linked above (summarizing the original issue): JS developers don't, broadly speaking, document expected exception behavior, so there's a chicken and egg problem where trying to implement checked-exception types would go against the grain of the current ecosystem.

Use of the @throws JSDoc tag can be treated as a sort of demand signal for better exception handling. Poor @throws adoption indicates that the community doesn't want it. And without good @throws coverage, a throws keyword in TS wouldn't be very useful in the best scenario, and would be actively misleading at worst, giving devs the impression that they've handled the "expected" throw scenarios when they haven't.

All that said, I still think there could be a place for some limited ability to perform static analysis of exception/rejection handling. I originally found the previous issue when I enabled a linter rule that looks for unhandled Promise rejections, which overlaps with try/catch once await enters the picture. I was looking for a way to decorate some async function calls as being unable to throw or reject (think return Promise.resolve('static value')), which would let me build out exception-safety from the bottom up, slowly. Maybe this could work if we split the feature into declaring or asserting throws-type (basically the @throws JSDoc tag), with a separate keyword or directive for enabling exception checking:

/** unsafe-assertion-that-this-throws-strings */
function throwsSometimes(): number {
  if (Math.random() < 0.5) { throw 'nope!'; }
  return (Math.random() < 0.5) ? 0 : 1;
}

/** unsafe-assertion-that-this-throws-never */
function throwsNever(): number { return JSON.parse('2'); }

/** checked-assertion-that-this-throws-never */
function maybeSafe(): number {
  return throwsSometimes() || throwsNever(); // error, unhandled throws-strings does not match declared throws-never
}

Note that this is a different scope from what was discussed in the checked-exceptions section of https://github.com/microsoft/TypeScript/issues/13219#issuecomment-1515037604. I'm trying to statically analyze that explicit/declared throws propagate correctly up the chain, and importantly, to limit where those checks are performed. I want to be able to decorate one function as not calling functions with decorated/expected exceptions outside of a try block -- to annotate one function as "exception-prone" and another as "bad at exception handling". I think there's value in that even if most library functions I call don't (currently) have their expected exceptions documented. (In Ryan's terminology, this requires "option one", unannotated functions are assumed not to throw anything.)

thw0rted avatar Mar 04 '24 18:03 thw0rted

Poor @throws adoption indicates that the community doesn't want it.

I don't know if this is true. Annotating with @throws doesn't actually enforce anything. So its value is only to document and therefore solves a different problem than checked exceptions (prevent unhandled exceptions). For those that document their code, I've found it very common to use @throws. But people aren't going out of their way to annotate.

A static analysis solution does seem to be the best. And leveraging @throws (for those who do use it) feels like a natural solution. And I agree if it's omitted, it's assumed it doesn't throw. But it would also be easy to have a linter tell you you're missing the annotation of a function that has a "throw" in its body (or a function it calls) - for those that do care.

kvenn avatar Mar 04 '24 18:03 kvenn

How about instead of all this, in your code you just return an Error, instead of throwing altogether. This is more reliable, easily supported by runtime behavior, requires less syntax to handle, already works in TypeScript, and wouldn't require massive refactoring if adopting a Result return type paradigm.

function wow(value: unknown) {
  if (typeof value === 'string') return new StringNotSupportedError();
  
  return { value: 1234 };
}

const result = wow('haha');

// @ts-expect-error
result.value; // TS error, you have to narrow the type

if (result instanceof Error) {
  // Handle the error
  return;
}

console.log(result.value); // Good!

It forces you to handle errors. Seems pretty similar to what people are asking for. I know that actually throwing behaves differently, but I wonder actually how much this would suffice.

I find that the more I think about this, the more I care about it only in my application source code. I'm not all that worried about third party libraries. I don't think there's been a single time where I wished a library had an error documented. Usually good type definitions avoid runtime errors that are worth catching. I also wonder if errors are even suitable for the things I have in mind. Things like validation contain useful state that don't really make sense to wrap in an error, and should instead just be included in a return value.

My questions are, what are the real world use-cases here? How do you guys actually see yourselves using a feature like this in practice? What errors do you have to explicitly handle with a try/catch, and do these instances occur frequently? How would the added type information help you?

KashubaK avatar Mar 04 '24 20:03 KashubaK

chicken and egg problem

I don't see it that way.

First step should be to implement typechecking of throw types the same way as return types. Then add throw types to standard library definitions which are part of TypeScript.

Then the library authors could just regenerate their definitions like always and have the throw types inferred the same way return types are inferred when you don't specify them. For backward compatibility, functions without a throw type would be treated as throw any or throw unknown, so if a library depends on another library which haven't been updated yet, it just gets it's own throw types inferred as unknown.

phaux avatar Mar 04 '24 20:03 phaux

"What if TS had typed/checked exceptions" is off-topic here; this is not a place to re-enact #13219

RyanCavanaugh avatar Mar 04 '24 20:03 RyanCavanaugh

"What if TS had typed/checked exceptions" is off-topic here

"Talk about exceptions here" 🤔

ETA: any chance the TS team would consider enabling the GitHub "Discussions" feature for posts like these? Issues are terrible at capturing long-running discussions because once there are too many comments, context gets lost behind the "Load more..." link and search breaks down.

thw0rted avatar Mar 04 '24 20:03 thw0rted

I agree that the topic of this thread is unclear about what's already been litigated, but it has already been extensively litigated (even if I'm bummed about the result). I think this thread was intended to be more of "other options, now that that decision has been made"

bensaufley avatar Mar 04 '24 21:03 bensaufley

I only found this issue after the previous one was closed.

My usecase was:

I wanted to enforce that a handler function will throw only HttpErrors:

type Handler<T, E extends HttpError<number>> = (req: Request) => T throw E

and I wanted to infer possible responses and their status codes based on what the function actually throws:

type handlerResponse<H extends Function> = 
  H extends (...args: any[]) => infer T throw HttpError<infer N>
    ? TypedResponse<200, T> | TypedResponse<N, string>
    : never

phaux avatar Mar 05 '24 00:03 phaux

I wonder if a simple util function could suffice.

function attempt<E extends Error,  T>(cb: () => T, ...errors: E[]): [T | null, E | null] {
  let error: E | null = null;
  let value: T | null = null;
  
  try {
    value = cb();
  } catch (err) {
    const matches = errors.find(errorClass => err instanceof errorClass);
    
    if (matches) {
      error = err;
    } else {
      throw err;
    }
  }
  
  return [value, error];
}

class StringEmptyError extends Error {}

function getStringLength(arg: string) {
  if (!arg.trim()) throw new StringEmptyError();
  
  return arg.length;
}

// Usage:

const [length, error] = attempt(() => getStringLength(" "), StringEmptyError);
// if error is not a StringEmptyError, it is thrown

if (error) {
  // error is a StringEmptyError
}

This is just an idea. It would need to be improved to handle async functions. It could also probably be changed to compose new functions to avoid repeating callbacks and errors, for example:

class StringEmptyError extends Error {}
class SomeOtherError extends Error {}

function getStringLength(arg: string) {
  if (!arg.trim()) throw new StringEmptyError();
  
  return arg.length;
}

// ... Assuming `throws` is defined
const getStringLength = throws(
  (arg: string) => {
    if (!arg.trim()) throw new StringEmptyError();
  
    return arg.length;
  },
  StringEmptyError,
  SomeOtherError
);

// Same usage, but a bit simpler

const [length, error] = getStringLength(" ");
// if error is not a StringEmptyError or SomeOtherError, it is thrown

if (error) {
  // error is a StringEmptyError or SomeOtherError
}

Kashuab avatar Mar 11 '24 22:03 Kashuab

That's a handy wrapper for, uh, turning TS into Go I guess? (There are worse ideas out there!) But I can't figure out how this helps with static analysis to enforce error checking. In particular, it looks like attempt(...) can only ever return [T,null] | [null,E] but TS isn't able to take advantage of that with flow-control based narrowing.

thw0rted avatar Mar 11 '24 23:03 thw0rted

My example wasn't meant to be perfect. It was just an idea on how to accomplish some way of better error handling. I've since improved the approach and implemented a way to enforce that errors are caught.

Example:

class StringEmptyError extends Error {}
class SomeOtherError extends Error {}

const getStringLength = throws(
  (arg: string) => {
    if (!arg.trim()) throw new StringEmptyError();

    return arg.length;
  },
  StringEmptyError,
  SomeOtherError
);

const length = getStringLength(' ')
  .catch(SomeOtherError, err => console.error(err))
  .catch(StringEmptyError, err => console.error(err));

console.log(length); // would be undefined in this case, it hits StringEmptyError

See CodeSandbox for a working throws implementation. src/throws.ts

If you don't add .catch(Error, callback) for each required error, you cannot access the original function's return value, and the function won't even be called. All errors are typed as expected. There are probably bugs and ways to improve it, I didn't take too much time here. This is definitely not compatible with async functions. Just wanted to prove that something like this is feasible.

Update: I also took the liberty of publishing this in a ts-throws NPM package. If anyone is interested in this feel free to try it out and add suggestions/issues on the repo: https://github.com/Kashuab/ts-throws

After some further development on this there are some obvious problems. But I think they can be addressed.

UPDATE 2: I've added more improvements to ts-throws to handle async functions and fixed quite a few bugs. It's in a pretty solid spot and I imagine it would work for a lot of developers. Check out the README for latest usage examples! Would love to hear some feedback.

Kashuab avatar Mar 12 '24 01:03 Kashuab

@kvenn I've tried my hand at an eslint plugin: https://github.com/Kashuab/eslint-plugin-checked-exceptions/tree/main

It introduces two rules:

  • uncaught-errors

Checks to see if a function you're calling has a @throws JSDoc annotation. If you don't wrap the function call in a try/catch it will output an error.

  • undocumented-errors

Warns you if a function has a throw statement without a corresponding @throws annotation. Matches based on what you're throwing, in case you have custom error classes.

Check out the tests for examples and what it covers. It's been a while since I looked at this, but I remember it being a bit buggy (i.e. nested branches, complicated logic) so there's a ton of room for improvement. I might take another look to improve it.

Side note - the README suggests you can install it from NPM, this is not the case haha.

(This is my work GH account, dunno why I have a separate one but oh well. I previously contributed here as @KashubaK)

This is really neat, btw I would suggest to not enforce try catch, because it's legit to ignore the error and let it propagate without putting eslint comments to disable the rule everywhere. Instead I would propose to force the user to annotate a function that is not catching an error with a @throws as well, this way the user can choose to ignore errors but at least the function openly declares that it may @throws.

raythurnvoid avatar Mar 13 '24 01:03 raythurnvoid

We can always use and wrap something like Rest style Result to handle error types, but long as actual throw error types are not part of TS this is just extra layer hack (same as trying to handle this on JSDoc) Easy things are propably just utilize Promise generics for Error Promise<string, TypeError>. Also adding throws keyword return type would also make sense

function hello(arg: unknown): string throws<TypeError> {}

... and have defaults like throws<any> or throws<unknown> based on TS settings. or maybe more compatible return type setup would be actually string & throws<TypeError> ?

mharj avatar Mar 17 '24 15:03 mharj

I'd like to re-plug a library I put together, since it's more refined than the examples I posted before. It lets you wrap a given function with enforced error catching, using syntax with similar verbosity when compared to a function using throws proposal and try/catch

  • Handle each error case with a separate callback, improved flow control vs. try/catch
  • Consumers don't need to import error classes
  • Everything is typed properly, auto-complete of catch* methods is available and they do get narrowed down so you don't have duplicates
  • No Result, but changes the return type to T | undefined if a checked error is thrown
  • No known bugs as of this comment
import { throws } from 'ts-throws';

class StringEmptyError extends Error {}
class NoAsdfError extends Error {}

const getStringLength = throws(
  (str: string) => {
    if (!str.trim()) throw new StringEmptyError();
    if (str === 'asdf') throw new NoAsdfError();
    
    return str.length;
  },
  { StringEmptyError, NoAsdfError }
);

/*
  `throws` will force you to catch the provided errors.
  It dynamically generates catch* methods based on the object of errors
  you provide. The error names will be automatically capitalized.
*/

let length = getStringLength(' ')
  .catchStringEmptyError(err => console.error('String is empty'))
  .catchNoAsdfError(err => console.error('String cannot be asdf'));

// length is undefined, logged 'String is empty'

length = getStringLength('asdf')
  .catchStringEmptyError(err => console.error('String is empty'))
  .catchNoAsdfError(err => console.error('String cannot be asdf'));

// length is undefined, logged 'String cannot be asdf'

length = getStringLength(' ')
  .catchStringEmptyError(err => console.error('String is empty'))

// Only one error caught, `length` is:
// { catchNoAsdfError: (callback: (err: NoAsdfError) => void) => number | undefined }
// Function logic not invoked until last error is handled with `.catch`

length = getStringLength('hello world')
  .catchStringEmptyError(err => console.error('String is empty'))
  .catchNoAsdfError(err => console.error('String cannot be asdf'));

// length is 11

One improvement might be error pattern matching for things like throw new Error('Some custom message'), this would help with wrapping third-party functions where their exception classes aren't public/exported

I think the only advantage that a native throws keyword would have over something like this would be conditionals (i.e. extends in a throws definition, function overrides, etc.) This solution doesn't seem like a hack to me, since it accomplishes the critical goal of forcing consumers of a given function to catch specific errors. I also prefer this catch-callback approach, it's cleaner than having to narrow error types manually within a catch block in most scenarios.

Kashuab avatar Mar 24 '24 21:03 Kashuab

I'd like to re-plug a library I put together, since it's more refined than the examples I posted before. It lets you wrap a given function with enforced error catching, using syntax with similar verbosity when compared to a function using throws proposal and try/catch

  • Handle each error case with a separate callback, improved flow control vs. try/catch
  • Consumers don't need to import error classes
  • Everything is typed properly, auto-complete of catch* methods is available and they do get narrowed down so you don't have duplicates
  • No Result, but changes the return type to T | undefined if a checked error is thrown
  • No known bugs as of this comment
import { throws } from 'ts-throws';

class StringEmptyError extends Error {}
class NoAsdfError extends Error {}

const getStringLength = throws(
  (str: string) => {
    if (!str.trim()) throw new StringEmptyError();
    if (str === 'asdf') throw new NoAsdfError();
    
    return str.length;
  },
  { StringEmptyError, NoAsdfError }
);

/*
  `throws` will force you to catch the provided errors.
  It dynamically generates catch* methods based on the object of errors
  you provide. The error names will be automatically capitalized.
*/

let length = getStringLength(' ')
  .catchStringEmptyError(err => console.error('String is empty'))
  .catchNoAsdfError(err => console.error('String cannot be asdf'));

// length is undefined, logged 'String is empty'

length = getStringLength('asdf')
  .catchStringEmptyError(err => console.error('String is empty'))
  .catchNoAsdfError(err => console.error('String cannot be asdf'));

// length is undefined, logged 'String cannot be asdf'

length = getStringLength(' ')
  .catchStringEmptyError(err => console.error('String is empty'))

// Only one error caught, `length` is:
// { catchNoAsdfError: (callback: (err: NoAsdfError) => void) => number | undefined }
// Function logic not invoked until last error is handled with `.catch`

length = getStringLength('hello world')
  .catchStringEmptyError(err => console.error('String is empty'))
  .catchNoAsdfError(err => console.error('String cannot be asdf'));

// length is 11

One improvement might be error pattern matching for things like throw new Error('Some custom message'), this would help with wrapping third-party functions where their exception classes aren't public/exported

I think the only advantage that a native throws keyword would have over something like this would be conditionals (i.e. extends in a throws definition, function overrides, etc.) This solution doesn't seem like a hack to me, since it accomplishes the critical goal of forcing consumers of a given function to catch specific errors. I also prefer this catch-callback approach, it's cleaner than having to narrow error types manually within a catch block in most scenarios.

i would highly discourage leveraging on throw to try mimic Go/Rust way of handling errors because each throw has a massive performance impact and this can easily get out of hand if overused + this forces you to wrap each function in a "throws" function that also adds a very small overhead.

I would suggest returning the errors instead of throwing them.

raythurnvoid avatar Mar 27 '24 22:03 raythurnvoid

i would highly discourage leveraging on throw to try mimic Go/Rust way of handling errors because each throw has a massive performance impact and this can easily get out of hand if overused + this forces you to wrap each function in a "throws" function that also adds a very small overhead.

I would suggest returning the errors instead of throwing them.

One of the benefits here is that it's plug-and-play with functions that already throw. You don't need to modify the function at all, just wrap it (this still introduces a breaking change for consumers, as would returning an error.)

Your suggestion would require refactors whose cost outweighs the value of performance in most scenarios. If you are running first-party functions in a loop where error handling performance becomes significant, those functions shouldn't throw to begin with as you suggested. My solution does not aim to address those use-cases.

My goal was to augment native error checking capabilities, making it more convenient/type-safe without needing to change the "throwing" code. Any overhead introduced would be negligible in 90% of use-cases, and in the latter 10% the library would not be applicable. That being said, I'll benchmark the library and see where optimizations can be made.


Update: I'm actually glad this got brought up. I put together some benchmarks and discovered my implementation was a bit over-complicated. Originally, throws wrapped functions were ~63% slower than a normal try/catch, but I've since refactored my approach to get it down to ~25% slower. It's still a considerable difference, but much better and easier to weigh.

In this benchmark, these were the results:

Thrower runs per second: 423095
Returner runs per second: 480135
ts-throws wrapped runs per second: 313880

Kashuab avatar Mar 28 '24 01:03 Kashuab

i would highly discourage leveraging on throw to try mimic Go/Rust way of handling errors because each throw has a massive performance impact and this can easily get out of hand if overused + this forces you to wrap each function in a "throws" function that also adds a very small overhead. I would suggest returning the errors instead of throwing them.

One of the benefits here is that it's plug-and-play with functions that already throw. You don't need to modify the function at all, just wrap it (this still introduces a breaking change for consumers, as would returning an error.)

Your suggestion would require refactors whose cost outweighs the value of performance in most scenarios. If you are running first-party functions in a loop where error handling performance becomes significant, those functions shouldn't throw to begin with as you suggested. My solution does not aim to address those use-cases.

My goal was to augment native error checking capabilities, making it more convenient/type-safe without needing to change the "throwing" code. Any overhead introduced would be negligible in 90% of use-cases, and in the latter 10% the library would not be applicable. That being said, I'll benchmark the library and see where optimizations can be made.

Update: I'm actually glad this got brought up. I put together some benchmarks and discovered my implementation was a bit over-complicated. Originally, throws wrapped functions were ~63% slower than a normal try/catch, but I've since refactored my approach to get it down to ~25% slower. It's still a considerable difference, but much better and easier to weigh.

In this benchmark, these were the results:

Thrower runs per second: 423095
Returner runs per second: 480135
ts-throws wrapped runs per second: 313880

The additional overhead is coming from generating the call stack when new Error() is called. That should be avoided as much as possible in frequently invoked code. The way I see it throw new Error() equals panic in rust/go, if your goal is to return recoverable errors you should use some custom class perhaps. However this means lot refactoring is necessary and not being plug n play.

So as a rule a would avoid to use the error class for expected situations, eg: the string in input is empty and it shouldn't be.

raythurnvoid avatar Mar 28 '24 23:03 raythurnvoid