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

Error propagation #16

Closed
nikolalukovic opened this issue Aug 16, 2024 · 6 comments
Closed

Error propagation #16

nikolalukovic opened this issue Aug 16, 2024 · 6 comments

Comments

@nikolalukovic
Copy link

I think the syntax for error propagation should be a part of this proposal. It would be nice to be able to leave the error handling to the caller (unless top level ofc) and just grab the result if it's successful.

Regular call:

const [error, result] = try someMethod();

With error propagation:

const result = try? someMethod();
@arthurfiorette
Copy link
Owner

You are overthinking...


Error propagation:

const result = someMethod();

@arthurfiorette arthurfiorette closed this as not planned Won't fix, can't repro, duplicate, stale Aug 16, 2024
@nikolalukovic
Copy link
Author

Sure. The syntax is the least important thing about it right now as long as it's clearly defined how it's going to work. Though I'd much rather it be explicit with try?.

@DScheglov
Copy link

DScheglov commented Aug 16, 2024

@arthurfiorette
backend in TypeScript needs a typed errors.

As instance we need to be able to express something like that:

interface IUserService {
  register(userData: UserData): Promise<Result<User, UserRegistrationError>>;
}

where UserRegistrationError is a string literal union:

export type UserRegistrationError =
  | "ERR_FORBIDDEN_EMAIL_DOMAIN"
  | "ERR_USER_IS_ALREADY_REGISTERED"
  | "ERR_PASSWORD_TOO_WEAK"
  | "ERR_PASSWORD_TOO_SHORT"
  | "ERR_PASSWORD_TOO_LONG"

In the same time we understand that register method also can throw, as instance if DB operation fails in some reason.

Why do we need that?

We are using GraphQL, so we generate the TS-typing for the GQL interface (aka mutations, types, etc),
and naturally we'd like to have a build-time proof that our business logic doesn't violate declared interface
returning some unexpected error.

We are not unique, so there are a lot solutions like fp-ts or effect-ts that exposes type Either (or equal)
that matches the ADT contracts described for Haskel types.

The best what we currently can have looks like the following:

class UserService {
 // -- snip --
 register(userData: UserData) {
   return Do(async function* () {
     yield* await this.dataPolicy.validateEmail(userData.email);
     yield* await this.dataPolicy.validatePassword(userData.password);
     const passwordHash = await this.cryptoService.generatePasswordHash(userData.password);

     const user = yield* await this.users.create({ ...userData, passwordHash });
     
     return user;
   }); 
 }
}

where this.dataPolicy, this.cryptoService and this.users are injected via the constructor

interface IDataPolicy {
  validateEmail(email: string): Promise<Result<void, "ERR_FORBIDDEN_EMAIL_DOMAIN">>;
  validatePassword(password: string): Promise<Result<void,  "ERR_PASSWORD_TOO_WEAK" | "ERR_PASSWORD_TOO_SHORT" | "ERR_PASSWORD_TOO_LONG">>
}
interface IUserRepo {
  create(userData: CreateUserData): Promise<Result<User, "ERR_USER_IS_ALREADY_REGISTERED">>
}

So, yield* in the example above does exactly that @nikolalukovic requested in this issue:

  • if function returns (and promise resolves) to Result and this Result is Err, yield* returns this Err
  • if function return Result that is Ok -- it just "unpacks" the value packed inside of Ok

The Do function reflects "returned" (actually yield-ed) errors to its return type.

In the same moment the possible exceptions are absolutely ignored here -- they will be
cautch by Error Boundary on the interface layer and reported to Sentry.

There are cases when we need to transform the Err value. For this reason themapErr
method is used, sometimes chainErr.

From the JavaScript point of view all of that doesn't make any sense: the errors couldn't be typed in any case.
But from TS point of view -- we definitely want to get generators rid, like we got it for promises (recall, in "old times"
we used the generators to write async/await code).

Current proposal improves error handling based on the untyped errors and seems to work for exceptions,
but for TypeScript it doesn't bring too much new: currently we can have something like doTry function.

We need more then just untyped error handling

@arthurfiorette
Copy link
Owner

backend in TypeScript needs a typed errors.

That's not what this proposal is trying to acheive neither wants to do so.

@DScheglov
Copy link

@arthurfiorette

yes, exactly, and that is a main problem with the proposal.

TypeScripts also needs the Symbol.result and it needs try? and try!, but these symbol and operators has a different meaning.

I've written a little bit more here: microsoft/TypeScript#56365 (comment)

@arthurfiorette
Copy link
Owner

yes, exactly, and that is a main problem with the proposal.

No its not.

image

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

No branches or pull requests

3 participants