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

Include a type parameter for error type in Stream #145

Open
TylorS opened this issue Oct 5, 2017 · 3 comments
Open

Include a type parameter for error type in Stream #145

TylorS opened this issue Oct 5, 2017 · 3 comments

Comments

@TylorS
Copy link
Member

TylorS commented Oct 5, 2017

Currently the types allow only the event values via Stream<A>. What if we also included the error type Stream<A, B>?

Reference TylorS/typed-stream#1
cc @goodmind

@briancavalier
Copy link
Member

briancavalier commented Oct 13, 2017

I think we could consider this now that we've clarified the distinction between application errors and stream failures. The new type parameter would be a stream failure type, rather than an application error type. For application errors, I think we should continue to point folks toward using an Either, which is already possible today:

const s: Stream<Either<string, MyCustomApplicationError>> = ...

vs a future where it's possible to be explicit about the stream failure type:

const s: Stream<Either<string, MyCustomApplicationError>, WebSocketClosedUnexpectedlyError> = ...

Those names are probably longer than what you'd typically encounter :) Even so, the type signature starts to get a little unwieldy--hopefully folks could lean on type inference most of the time.

We could potentially default the new type parameter to Error in both Flow and TS to avoid a breaking change, if we decide to do this before 2.0.

Thoughts?

@Frikki
Copy link
Member

Frikki commented Oct 13, 2017

I like the long names.

@christianbradley
Copy link

christianbradley commented Apr 29, 2019

I think we could consider this now that we've clarified the distinction between application errors and stream failures. The new type parameter would be a stream failure type, rather than an application error type. For application errors, I think we should continue to point folks toward using an Either, which is already possible today:

const s: Stream<Either<string, MyCustomApplicationError>> = ...

This seems a bit confusing to me... as it does not indicate the terminal nature of a stream exception.

Here's a general idea of the route I'd go...

class StreamFailure {
  private constructor(readonly cause: Error) {}
  static of: (e: Error) { return new StreamFailure(e) }
  static is(a: unknown): a is StreamFailure { return a instanceof StreamFailure }
}

type SinkException<A> = StreamFailure | A

interface Sink<L,A> {
  next(a: A): void
  error(e: SinkException<L>): void
  complete(): void
}

interface Stream<L,A> {
  run(sink: Sink<L,A>): Disposable
}

const foldSinkException = <A,B>(
  f: (e: StreamFailure) => B,
  g: (e: A) => B
) => (e: SinkException<A>) => StreamFailure.is(e) ? f(e) : g(e)
// example usage
type AppException = 
  | { tag: 'tag1', data: string }
  | { tag: 'tag2', data: number }

const myStream: Stream<AppException, void> = getMyStream()

myStream.run({
  next: () => console.log("tick"),
  error: foldSinkException<AppException, void>(
    l => console.error(l.cause),
    a => {
      switch(e.tag) {
        case 'tag1': return console.log(e.data)
        case 'tag2': return console.log(e.data.toString())
      }
    }
  ),
  complete: () => console.log('done')
})

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

4 participants