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

Adding timeout support to Data.Conduit.Network #382

Open
chrisdone opened this issue Jul 27, 2018 · 1 comment
Open

Adding timeout support to Data.Conduit.Network #382

chrisdone opened this issue Jul 27, 2018 · 1 comment

Comments

@chrisdone
Copy link

I whipped up this yesterday evening https://gist.github.com/chrisdone/e25dac215481d59f730f7c06df031cd1

The use-case is simply that Data.Conduit.Network already lets you write clean server code, but there isn't a built-in way to stop connections being opened forever. You could have a timeout on the main thread, but what if someone simple has a very slow connection? Hence, we put a timeout on the individual send/recv commands.

It works fine - but:

  1. Do you have an opinion on timeout? Apparently, its docs say that it safely interrupts socket calls. It should be fine, but I'm used to all things conduit really handling error and tricky conditions cleanly, so it's worth being picky.
  2. Are you interested in adding timeout support into Data.Conduit.Network like this? Either in appData directly as a HasTimeout, for example, or by adding an extra param to the appSink and appSource functions. And also they can return e.g. WriteTimeout, ReadTimeout and RemoteClosed for example. I just used two constructors because it was convenient for my case.

To avoid breaking existing code we could add new functions like appSourceTimeout, etc. My Connector type was in anticipation of other options for my use-case.

I'll open a PR if interested. 👌

@snoyberg
Copy link
Owner

Using timeout on socket operations should in general work, I have no objection to that.

I'd be in favor of either adding new functions that apply timeouts to each read/write operation, or adding a new configuration value to set such optional timeouts for the existing functions. I'm not in favor of modifying the existing type signatures for backwards compat reasons.

Note that if you choose to go the configuration route, these changes would probably make more sense in streaming-commons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants