Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail-safe mode: return without throwing on failure #157

Open
Deilan opened this issue Jun 8, 2024 · 3 comments
Open

Fail-safe mode: return without throwing on failure #157

Deilan opened this issue Jun 8, 2024 · 3 comments

Comments

@Deilan
Copy link

Deilan commented Jun 8, 2024

Currently, when a client.invoke results in a failure, an error object is constructed and thrown, adhering to the fail-fast principle. I would like to know if there is a mode or configuration option available where the result is returned without constructing and throwing an error object (fail-safe).

Specifically, I'm looking for a way to handle errors where the method returns a result object with an error flag or code, allowing the caller to handle the error without exceptions being thrown. Is such a mode supported or planned for future releases?

@eilvelia
Copy link
Owner

eilvelia commented Jun 8, 2024

Hm, I see that this can be useful in some cases. It can be implemented in terms of client.invoke:

const invoke = req => client.invoke(req)
  .catch(e => {
    if (e instanceof tdl.TDLibError)
      return { _: e._, code: e.code, message: e.message }
    throw e
  })

This would allow a Go-style error handling on the spot.

A similar one is used here: https://github.com/LyoSU/quote-bot/blob/6d740e8b0a94c1b2e338443c91da3090cc889d68/helpers/tdlib/index.js#L25 (it's a bit simpler)

If you use TypeScript, typing this is a more difficult part, but still should possible using the following:

import type * as Td from 'tdlib-types'

const invoke: <T extends Td.$FunctionName>(
  query: { readonly _: T } & Td.$FunctionInputByName[T]
) => Promise<Td.$FunctionResultByName[T] | Td.error> = (
  req: any
): any =>
  client.invoke(req).catch(e => {
    if (e instanceof tdl.TDLibError)
      return { _: e._, code: e.code, message: e.message }
    throw e
  })

I'm not sure adding another function to client with this behavior is worth it though.

@Deilan
Copy link
Author

Deilan commented Jun 9, 2024

Thank you for the samples. Having fail-safe mode as a first-class option is important for a few reasons:

  1. Performance: Throwing and catching errors can be significantly slower compared to returning and checking results, potentially impacting performance.
  2. Debugging Experience: When developers set breakpoints on caught exceptions, they may encounter numerous interruptions due to exceptions being thrown frequently, making debugging more cumbersome.

The current fail-fast mode is a great default. I'm suggesting an option to switch to fail-safe mode, similar to how there is an option to create a bare client instead of a full one.

@eilvelia
Copy link
Owner

eilvelia commented Jun 9, 2024

Performance seems to be a moot point; the extra cost of one promise rejection should be negligible, especially compared to an API request. Probably the largest benefit is convenience of not having to define such functions yourself.

When one starts using TDLib, it may feel tempting to base logic on handling API errors, but this approch may be misguided and is criticized by the TDLib's author (levlam's quote, "Error-driven development is a wrong way"). If requests fail with errors in infrequent exceptional cases only, the breakpoint-on-caught-exceptions case also seems moot (as well as performance and need for this feature).

Implementing it as an option would complicate TypeScript typing (and generally I feel that it's better design to make a separate function, considering that it changes the invoke interface significantly). The way to implement this is to add a function like client.invokeNoReject. I'm still not fully convinced this feature should be added though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants