Feedback

[gcanti]

Based on the following (self imposed) requirements:

• the library shouldn't be tied to io-ts
• the library shouldn't be tied to cross-fetch
• decoding shouldn't be optional (not recommended but you can always resort to a dummy decoder like E.right)
• a handler for unexpected errors shouldn't be optional

I would propose an API like this:

import * as E from 'fp-ts/lib/Either'
import { ReaderEither } from 'fp-ts/lib/ReaderEither'
import { TaskEither } from 'fp-ts/lib/TaskEither'

type _Fetch = typeof fetch

export interface Fetch extends _Fetch {}

export interface Decoder<E, A> extends ReaderEither<unknown, E, A> {}

type Status = 200 | 400 | 500 // etc...

export type FetcherError =
  | { readonly type: 'JsonDeserializationError'; readonly details: unknown }
  | { readonly type: 'HandlerNotSetError'; readonly status: number }

export interface Fetcher<S extends Status, E, A> {
  readonly input: RequestInfo
  readonly handlers: Record<S, Decoder<E, A>>
  readonly onUnexpectedError: (error: FetcherError) => E.Either<E, A>
  readonly init?: RequestInit
}

export function make<S extends Status, E, A>(
  input: RequestInfo,
  handlers: Record<S, Decoder<E, A>>,
  onUnexpectedError: (error: FetcherError) => E.Either<E, A>,
  init?: RequestInit
): Fetcher<S, E, A> {
  return { input, onUnexpectedError, handlers, init }
}

export function toTaskEither(fetch: Fetch): <S extends Status, E, A>(fetcher: Fetcher<S, E, A>) => TaskEither<E, A> {
  // sketch implementation
  return <S extends Status, E, A>(fetcher: Fetcher<S, E, A>) => async () => {
    const isStatus = (status: number): status is S => fetcher.handlers.hasOwnProperty(status)
    const response = await fetch(fetcher.input, fetcher.init)
    const status = response.status
    if (isStatus(status)) {
      try {
        const contentType = response.headers.get('content-type')
        const body: unknown =
          contentType?.includes('application/json') !== undefined ? await response.json() : await response.text()
        const handler = fetcher.handlers[status]
        return handler(body)
      } catch (details) {
        return fetcher.onUnexpectedError({ type: 'JsonDeserializationError', details })
      }
    } else {
      return fetcher.onUnexpectedError({ type: 'HandlerNotSetError', status })
    }
  }
}

Example

import * as t from 'io-ts'
import { failure } from 'io-ts/lib/PathReporter'
import { flow } from 'fp-ts/lib/function'
import { fetch } from 'cross-fetch'

const taskify = toTaskEither(fetch)

type MyError = { readonly type: 'unexpectedError' } | { readonly type: 'decodeError'; readonly message: string }

const unexpectedError: MyError = { type: 'unexpectedError' }

const decodeError = (errors: t.Errors): MyError => ({
  type: 'decodeError',
  message: failure(errors).join('\n')
})

const getFetcher = (url: string): Fetcher<200 | 400, MyError, string> =>
  make(
    url,
    {
      200: flow(
        t.type({ title: t.string }).decode,
        E.bimap(decodeError, user => user.title)
      ),
      400: flow(t.string.decode, E.mapLeft(decodeError))
    },
    () => E.left(unexpectedError)
  )

// te: TaskEither<Err, string>
const te = taskify(getFetcher('https://jsonplaceholder.typicode.com/posts/1'))

te().then(console.log)
/*
{ _tag: 'Right',
  right:
   'sunt aut facere repellat provident occaecati excepturi optio reprehenderit' }
*/

taskify(getFetcher('https://jsonplaceholder.typicode.com/posts'))().then(console.log)
/*
{ _tag: 'Left',
  left:
   { type: 'decodeError',
     message:
      'Invalid value undefined supplied to : { title: string }/title: string' } }
*/

[YBogomolov]

Thank you @gcanti for this proposal! I will play with your approach a bit and see if it suits my initial design goals.

I was thinking of the following scenarios when designed this package:

1. Programmer knows exactly all expected status codes – i.e., 200 | 400 | 401 | 403, – as well as payload type per each status. Then he/she can specify which statuses should be handled explicitly using .handle(code, handler, codec) – in such case handler should be inferred as a function receiving the same exact type of payload which was specified for the handled code.

2. Programmer can handle explicitly not every HTTP code if he/she cares only about a subset – say, only 200 and 204, and for any other codes some kind of fallback should be executed. In this case programmer just calls:

  fetcher
  .handle(200, cb1, codec1)
  .handle(204, cb2, codec2)
  .discardRest(() => something)

As far as I can see right now, your approach requires full coverage of possible status codes in make. I will explore if I can make your handlers parameter a Partial<Record<S, Decoder<E, A>>>, and make onUnexpectedError cover the rest cases.

[YBogomolov]

Another approach I was thinking of – make Fetcher a set of classes, each with different capabilities. Something similar to a Free monads or Scala's ZIO design. Such approach allows even greater API discoverability – e.g., .handle() could return a class which only has .handle and .discardRest methods, and .discardRest returns a class which only has .run method.

I was playing a bit with similar approach when explored possibility of session types implementation in TS: https://gist.github.com/YBogomolov/c5675b788a7046b6edf9e76ef7337af7. I really like the idea of discoverable APIs, when the compiler guides you towards correct consumption of the API.

[gcanti]

As far as I can see right now, your approach requires full coverage of possible status codes

You can handle as many status codes as you wish (full or partial coverage)

//
// README example
//

import * as t from 'io-ts'

const User = t.type({ name: t.string })
const Users = t.array(User)
const FourTwoTwo = t.type({ code: t.number, correlationId: t.string })

interface User extends t.TypeOf<typeof User> {}
interface FourTwoTwo extends t.TypeOf<typeof FourTwoTwo> {}

type GetUserResult =
  | { code: 200; payload: User[] }
  | { code: 400; payload: Error }
  | { code: 401; payload: [Error, string] }
  | { code: 422; payload: FourTwoTwo }

//                      v-- ensures total coverage                missing handlers ---v
const fetcher1: Fetcher<GetUserResult['code'], string, GetUserResult> = make('myurl', {}, () =>
  E.left<string, GetUserResult>('unexpected error')
)
// ^--- error: Type 'Fetcher<never, string, GetUserResult>' is not assignable to type 'Fetcher<200 | 400 | 401 | 422, string, GetUserResult>'

import { flow } from 'fp-ts/lib/function'
import { failure } from 'io-ts/lib/PathReporter'

const decodeError = (errors: t.Errors): string => failure(errors).join('\n')

const handleUsers = flow(
  Users.decode,
  E.bimap(decodeError, payload => ({ code: 200 as const, payload }))
)

const handleFourTwoTwo = flow(
  FourTwoTwo.decode,
  E.bimap(decodeError, payload => ({ code: 422 as const, payload }))
)

// partial coverage, fetcher2 is inferred as: Fetcher<200 | 422, string, GetUserResult>
const fetcher2 = make(
  'myurl',
  {
    200: handleUsers,
    422: handleFourTwoTwo
  },
  () => E.left<string, GetUserResult>('unexpected error')
)

// you could also specify which statuses should be handled explicitly using
// a type annotation
const fetcher3 = make<200 | 422 | 400, string, GetUserResult>(
  'myurl',
  {
    200: handleUsers,
    422: handleFourTwoTwo
  },
  () => E.left('unexpected error')
)
// ^--- error: Property '400' is missing in type ...

[gcanti]

Such approach allows even greater API discoverability

I agree, chainable APIs are nice for API discoverability, but there's no need for better discoverability with an API like make: configuration is done in one shot.

Isn't the discardRest signature problematic?

discardRest(restHandler: () => To)

What if I can't produce a To (say I'm fetching a User)?

p.s.

Btw I'm not sure about onUnexpectedError's signature, perhaps should simply be (error: FetcherError) => E

[YBogomolov]

Isn't the discardRest signature problematic?
What if I can't produce a To (say I'm fetching a User)?

I though of restricting To to have a monoid instance, so its unit could be returned in case of unexpected errors, making discardRest not required. But on the second thought I decided that providing a fallback is easier than constructing a monoid instance.

[YBogomolov]

@gcanti I would like to thank you once again for your feedback and suggested implementation. I played with it a little bit, and written the following: https://gist.github.com/YBogomolov/5de9290f6a7056f17a52b088ac298441

Main ideas:

1. The response might not contain data in body – e.g., for 204 its natural to pass the useful payload in headers (from my experience). Thus I removed all work with Response to the handlers.
2. As handlers now work with a raw response, the Decorder<E, A> is ReaderTaskEither<Response, E, A> now, as well as onUnexpectedError is ReaderTaskEither<FetcherError, E, A>.
3. Decoders for text & JSON could be exposed by default, decoder for headers has to be build manually. I played with some constructors, but none was really ergonomic.
4. I've added function extend, which adds new handlers to an existing fetcher (and updates its type). This allowed me to fulfil one of my initial design goals (which is implemented in current version of Fetcher class): user should be able to gradually refine type of Fetcher. This approach allows passing around partially built fetchers – say, in some "core" module you tell fetcher how to handle 400/401/403/500 error codes, and then in "app" module you tell it how to handle 200 code. In one of my commercial projects we used such approach and it proved its usefulness.

I would be glad if you take a look at it 🙏

[gcanti]

As handlers now work with a raw response, the Decorder<E, A> is ReaderTaskEither<Response, E, A>

👍 much better

onUnexpectedError is ReaderTaskEither<FetcherError, E, A>

Looks like JsonDeserializationError is unused now, what about onUnexpectedError: Decoder<E, A>?

export interface Fetcher<S extends Status, E, A> {
  ...
  readonly onUnexpectedError: Decoder<E, A>;
  ...
}

(maybe should be renamed)

Decoders for text & JSON could be exposed by default

👍

This approach allows passing around partially built fetchers

I'd say you pass around partially built handlers

export type Handlers<S extends Status, E, A> = Record<S, Decoder<E, A>>

// partially built handlers
const handlers: Handlers<400 | 100, Error, never> = {
  400: () => TE.left(new Error('400')),
  100: () => TE.left(new Error('100'))
}

const fetcher21 = extend(fetcher2, handlers)

Nitpick: now handlers can be overwritten, the name "extend" could be misleading, what about

//                                disallow overwrites ---v
export function extend<OldS extends Status, NewS extends Exclude<Status, OldS>, E, A>(
  fetcher: Fetcher<OldS, E, A>,
  handlers: Record<NewS, Decoder<E, A>>,
  onUnexpectedError?: ReaderTaskEither<FetcherError, E, A>
): Fetcher<OldS | NewS, E, A>

[YBogomolov]

@gcanti it really clicks now :) I've split extend into two implementations: extendWith, which allows changing type of the result (A => B), and extend, which acts as an endomorphism on fetcher's result type. And the implementation is much clearer. Please see this iteration: https://gist.github.com/YBogomolov/5de9290f6a7056f17a52b088ac298441

Nitpick: now handlers can be overwritten, the name "extend" could be misleading, what about

I totally forgot about the Exclude, shame on me. Thanks for the tip!

(maybe should be renamed)

I don't like the long name, but it's quite verbose, so I'd stick with it ¯_(ツ)_/¯

[gcanti]

which allows changing type of the result (A => B)

Isn't this map?

export function map<S extends Status, E, A, B>(fetcher: Fetcher<S, E, A>, f: (a: A) => B): Fetcher<S, E, B> {
  return {
    input: fetcher.input,
    handlers: pipe(fetcher.handlers, R.map(RTE.map(f))) as Record<S, Decoder<E, B>>,
    onUnexpectedError: pipe(fetcher.onUnexpectedError, RTE.map(f)),
    init: fetcher.init
  }
}

// not interesting since can be derived from `map` and `extend`
export const extendWith = <A, B extends A>(ab: (a: A) => B) => <
  OldS extends Status,
  NewS extends Exclude<Status, OldS>,
  E
>(
  fetcher: Fetcher<OldS, E, A>,
  handlers: Handlers<NewS, E, B>
): Fetcher<OldS | NewS, E, B> => extend(map(fetcher, ab), handlers)

[YBogomolov]

Isn't this map?

Yep, it is. I'll add map/mapLeft/bimap, they will be handy.

// not interesting since can be derived from map and extend

And extend could be expressed as extendWith(identity) 🙃 Still it makes API a bit more welcoming, IMHO.

I will test this vnext iteration of fetcher-ts for a bit and see how it feels from the point of end-user. Once again – thank you @gcanti for your collaboration on this project! The design you've suggested seems more streamlined with pipeable approach. I really like it.

[YBogomolov]

The resulting iteration: https://gist.github.com/YBogomolov/5de9290f6a7056f17a52b088ac298441

[vecerek]

This is a great thread! I've been thinking about a fetcher that could take a set of decoders as input in form of a tagged union (here and here) but never got to realize my ambition 😅 . I like the change that made onUnexpectedError a required param. There's always something unexpected that should be taken care of 🙂 That brings me to my question. If I were to implement a circuit breaker on top of this new API, how would I go about it? The circuit breaker should be configurable per client which might translate to per "instance" of a fetcher. Furthermore, the configuration should contain a threshold and a list of "exceptions" which can be response status codes, kinds of error (network timeout error, socket error, connection error, etc.). To achieve this, the "left" type should provide data that allows these cases to be identified. The circuit breaking is just an example, one might want to wrap the fetcher with a simple retry logic that also depends on the response status code because there's no point in retrying if the error is a client error.

Bonus question: would this fetcher work with the experimental modules of io-ts?