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

IO.read should not gobble errors silently #242

Closed
avsm opened this issue Jan 26, 2015 · 6 comments
Closed

IO.read should not gobble errors silently #242

avsm opened this issue Jan 26, 2015 · 6 comments
Labels

Comments

@avsm
Copy link
Member

avsm commented Jan 26, 2015

IO.read currently returns an option, with no ability to comment on the sort of error it's returning. This is a fairly big revision, but it needs to move to a model where errors and end-of-files are distinguished.

See #207

@Chris00
Copy link
Contributor

Chris00 commented Jan 28, 2015

What is the preferred scheme for error handling? Using something like Or_error, using Lwt.fail (nor really portable to async), polymorphic variants,…?

@talex5
Copy link
Contributor

talex5 commented Jan 31, 2015

Currently undecided, but I'm collecting comments and proposals here: mirage/mirage-www#274

(also, there's lots of discussion on the mailing list at present)

@avsm
Copy link
Member Author

avsm commented Feb 9, 2015

Gobbling errors like is is really, really harmful in larger scale uses of Cohttp. Running https://github.com/avsm/opam-mirror is causing undebuggable failures when running a large number of threads. This must be fixed before a 1.0 despite the interface breaking change I think /cc @rgrinberg

@avsm avsm added the API label Feb 9, 2015
@avsm avsm added this to the 1.0 Stable API milestone Feb 9, 2015
@SGrondin
Copy link
Contributor

SGrondin commented Feb 9, 2015

Agreed. I'm running a busy cohttp server and managing its crashes and errors is a pain.

@rgrinberg
Copy link
Member

@avsm I'm +1231241 on this change but why do you consider this a breaking change? I don't consider S.IO part of our stable API.

@SGrondin Can you please describe in a bit more detail. If it's graphic enough, it would encourage me to crack my knuckles and finally tackle this.

@yomimono
Copy link
Contributor

yomimono commented Apr 6, 2018

src/s.ml now has a module type Http_io with

val read : IO.ic -> [ `Eof | `Invalid of string | `Ok of t ] IO.t

, which seems to fulfill this request, so I'll close this issue. If this is incomplete or incorrect, please feel free to reopen.

@yomimono yomimono closed this as completed Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants