search

jeddeloh / rescript-apollo-client

ReScript bindings for the Apollo Client ecosystem
MIT License
126 stars 18 forks source link

Flesh out network error #11

Closed jeddeloh closed 4 years ago

jeddeloh commented 4 years ago

Apollo types the networkError property on ApolloError as Error | null which is kinda tragic. This is my initial exploration at fleshing things out. In an ideal world, network error would be something like this: https://github.com/elm/http/blob/2.0.0/src/Http.elm#L538

type Error
  = BadUrl String
  | Timeout
  | NetworkError
  | BadStatus Int
  | BadBody String

Unfortunately we don't have much ability to get this information after the fact. The types in @apollo/link-error are better: Error | ServerError | ServerParseError So tried working with that. It's wasn't clear to me from those names what they actually mean without digging into the internals of Apollo. If I were to rename them I'd do something like this: 

type networkError =
| FetchFailure(Js.Exn.t)
| BadStatus(int, ServerError.t)
| BadBody(ServerParseError.t)

Right now, I've messily pulled the types from the link-error bindings over and kept the names of the variants the same (Error | ServerError | ServerParseError), but I'm curious what anyone thinks of renaming the variants. This is one of the first cases where we're diverging from the TypeScript types, so I'm actually leaning that direction.

jeddeloh commented 4 years ago

I'm going to optimistically merge this. Let me know if you comments later!

jfrolich commented 4 years ago

I think providing better names is good!

jeddeloh commented 4 years ago

@jfrolich Thanks for the feedback! Made the change in #15