Promise of result--why or why not?

1

The recent ReScript Promise binding thread triggered an interesting question–what, if anything, is wrong with using Promise of result? Let me try to gather up the pros and cons I know about so far:

Pros

  • If used properly, can ensure no rejected promises at runtime
  • Guarantee no Promise flattening i.e. unsoundness
  • Can use polymorphic variant error types to compose together errors from many modules with precise control

Cons

  • Introduce a new wrapper for every value inside a Promise
  • Stack traces can be lost if we don’t use exceptions. However it’s easy to capture exceptions in the Error(...) case of a result

Thoughts?

6 Likes
2

Usually we try to keep our promises as simple as possible. We lean towards using result for domain relevant representation. If I/O (like async call to a database) fails, we normally try to handle it directly in a catch clause.

Recently we had a nasty bug due to promises and a misconception of Promise.all: We changed everything in that promise chain to resolve as a result to be able to handle any combination of failure correctly.
Readability suffered a but, but handling got corrected.

1 Like
3

I do really like the idea of using result inside promises, knowing statically what error types are possible are a big benefit. That said I haven’t used this in production yet, so I’m not aware of any pitfalls thus far

4

There’s prior art in this space, a lot of it in the Scala community, where they call it ‘bifunctor IO’. E.g. check out https://zio.dev/docs/overview/overview_index

In fact a promise of result is actually still a little short of a ‘pure functional’ async type, because it is an eagerly evaluated promise. But by emulating ZIO’s ‘reader monad’ we can actually make the promise lazily evaluated. ‘Reader monad’ is just a fancy way of saying ‘put it in a thunk’, i.e.

type t<'a, 'e, 'r> = 'r => Js.Promise.t<result<'a, 'e>>

We can define all the standard promise functions around this lazily-evaluated promise type and get all sorts of benefits–mainly, full control over when promises run, with what input data, and of course with guaranteed strongly typed errors with no promise rejections.

9 Likes
5

We’ve written a ton of code with this pattern, (actually, we started with this exact type a couple of years ago, then moved to r => Relude.IO.t('a, 'e), and most recently wrote our own GADT for it). We actually call it Zio.t because of how much it maps. I don’t know how much it aligns with the opinionated goals of ReScript but I can’t imagine working without it on a decently sized codebase anymore.

3 Likes
6

Lately I have been using Promise.t<Result.t<'a>> and the readability of the code really suffers vs catching and raise exceptions.

Sometimes you can even get types like Result.t<Promise.t<Result.t<'a, 'e>>, 'e> and it is very hard to understand the code 1 day after you write it

I wonder if we can have the best of both worlds by having rescript compiler track unhandled promise rejections automatically and warn about them. (Once we have a dedicated async/await/trycatch syntax, this can be done imo)

https://github.com/reason-association/reanalyze Already sort of does this. I wonder if that can be deeply integrated with rescript compiler

4 Likes
7

That’s a very interesting idea. It would make ReScript quite unique in not just the AltJS space but even relative to pretty much every compiler available today.

8

I’m not all caught up on the Promise plans, but for Unhandled Promise Exceptions, I think it depends on if using type Promise.t('a, 'e), we still have Promise.error… or if, as discussed, somehow the exception goes up into the 'e. It’s difficult because you can reject with a useful error value but anything below that promise can just throw at any point… so that type of 'e really needs to be something like type t('e) = | Cause('e) | Exception(Js.Exn.t).

We also now just avoid promises entirely because Promise.error is not very useful. We use callbacks for exceptions we actually want to track.

I know Relude and ReScript don’t have the same goals, but this is exactly what we used Relude.Void.t for. Say you have Promise.t('a, 'e), you handle the error with a Promise.catch… you end up with Promise.t('a, Void.t)… meaning the error is handled. At the “end” of your app logic when you need to go back into ffi, type it to only accept Promise.t('a, Void.t) and you know you have no unhandled errors. Could even call it Promise.handled if Void.t is too scary.

Result.t<Promise.t<Result.t<'a, 'e>>, 'e>

We had this problem a few times when we first started down this path. Generally the types will just stop you, but I think it mostly has to do with how confusing it is to have to do so much manual promise wrapping and unwrapping all of the time without any real help. Working directly with Promises of Results, we ended up with like 3x more code noise… completely unhelpful, masks logic, and invites errors.

We solved this, like I mentioned above, by creating a Zio.t type to represent it. Once we wrote helper functions specific to this type, like Zio.try and Zio.flatMap (I know, I know…) this issue completely disappeared. Outside of onboarding someone new to this pattern, it never comes up. (But, also, as I wrote above, we did stop using promises entirely.)

Usually our code is something like:

  10 lines of ffi (input)
1000 lines of application logic
  10 lines of ffi (output)

This isn’t so much a “ReScript/JavaScript” thing as much as it is a “App logic/Implementation details” thing. For us, promises only matter for ffi and are an annoying implementation detail forced on everywhere else. We optimize for the 1000 lines of constantly changing, and important to our business, app logic instead of the 10 lines of ffi we write once. Promise is not a very useful type for App logic. So turn it into a helpful type at input FFI and never thinking about the structure of it again until we need to undo that the output ffi.

7 Likes
9

Hey, maybe your approach should be the ReScript way, actually? Only using promises when you have no choice, otherwise defaulting to callbacks.

10

Well, I’d love it, haha. But I think it’d be very annoying to have to keep explaining to people. Promises are just too ubiquitous in js and they’re becoming more and more so with every new feature they add. :pensive: Also, in the end you can just choose not to use Promises, like we do.

2 Likes
11

Hi folks,

I have a related question about all this. I’m currently in a Node.JS backend function.
A lot of things can go wrong in my request handler. Some of these things I know upfront, so I tried to model it a bit.

open FirebaseFunctions.Logger
open FirebaseFunctions.Https

type hookErrorType =
  | InvalidRequestMethod
  | MissingRequestBody
  | MissingStripeHeader
  | ConfigurationError
  | WebhookSignatureVerificationFailed({err: Error.t})

// Custom exception used in the promise
exception HookError(hookErrorType)

let stripeSignatureHeader = "stripe-signature"

let getNonEmptyString = (s: string) => {
  switch Nullable.make(s)->Nullable.toOption {
  | None => None
  | Some(v) if v == "" => None
  | Some(v) => Some(v)
  }
}

let validateRequest = request => {
  if request->method != #POST {
    Promise.reject(HookError(InvalidRequestMethod))
  } else {
    let rawBodyOpt = request->rawBody->getNonEmptyString
    let stripeHeaderOpt = request->headers->Dict.get(stripeSignatureHeader)
    switch (rawBodyOpt, stripeHeaderOpt) {
    | (None, _) => Promise.reject(HookError(MissingRequestBody))
    | (Some(_), None) => Promise.reject(HookError(MissingStripeHeader))
    | (Some(rawBody), Some(stripeHeader)) => Promise.resolve((rawBody, stripeHeader))
    }
  }
}

let getEndpointSecret = () => {
  switch Common.stripeEndpointSecret->Nullable.make->Nullable.toOption {
  | None => Promise.reject(HookError(ConfigurationError))
  | Some(endpointSecret) => Promise.resolve(endpointSecret)
  }
}

let stripe = Stripe.make(Common.stripe_private_key)

let constructStripeEvent = (rawBody, signature, endpointSecret) => {
  try {
    let event =
      stripe->Stripe.constructEvent(~payload=rawBody, ~header=signature, ~secret=endpointSecret)->Ok

    if event->Nullable.make->Nullable.toOption->Option.isNone {
      Js.Exn.raiseError("Stripe event was null")
    } else {
      Promise.resolve(event)
    }
  } catch {
  | Js.Exn.Error(obj) => Promise.reject(HookError(WebhookSignatureVerificationFailed({err: obj})))
  }
}

let savePaymentEvent:Promise<unit> = (event: Stripe.webHookEvent) => {
  // Saving to Firebase, this will happen via a promise
  Promise.resolve()
}

// Main entry point
let paymentHook = onRequest(
  ~opts={
    region: Domain.firebaseRegion,
  },
  ~handler=async (request, response) => {
    try {
      let (rawBody, stripeHeader) = await validateRequest(request)
      let endpointSecret = await getEndpointSecret()
      let event = await constructStripeEvent(rawBody, stripeHeader, endpointSecret)
      logger->info("Stripe event", ~data={"event": event})
    } catch {
    | HookError(errorType) => () // deal with things I know could go wrong
    | exn => () // deal with unexpected errors
    }
  },
)

I tried to use promises all the way, because some steps in my pipeline need to be promises. Other stuff could be synchronous and use the Result type, but that doesn’t mix well the moment I need a promise.

Is it sensible to go this promise route? Any advice on this?

12

Hot take, but I have had success with using a polymorphic variant for the error type, it allows you to accumulate a wide range of possible errors as your promise value winds its way up the stack.
We use this for example, to project all the possible errors from graphql and apollo, and then many of the places where a resolver returned None along the way.

1 Like
13

Mind posting a (somewhat elaborate) example of this? I think that might be helpful.

14

I built a small library which I’m using in many of my projects: GitHub - dkirchhof/rescript-async-result

It’s just a wrapper for a promise<result<'a, 'b>> type

2 Likes
15

I like it! It’s basically a TaskEither. Now it just needs dependency injection and we have the Effect type from ts-effect.

1 Like