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

Expose end-of-stream from channels #247

Closed
mschuwalow opened this issue Sep 6, 2020 · 4 comments
Closed

Expose end-of-stream from channels #247

mschuwalow opened this issue Sep 6, 2020 · 4 comments

Comments

@mschuwalow
Copy link
Member

Currently whenever we read from a channel and it returns -1, we fail with an IOException using https://github.com/zio/zio-nio/blob/master/nio-core/src/main/scala/zio/nio/core/package.scala#L15.

This can be very inconvenient when writing for example a streams-based consumer as it makes it impossible to differentiate between the socket getting closed and an error, without exploiting implementation details.
I propose we should instead model read as def read(b: ByteBuffer): IO[Option[IOException], Int], where fail(None) signals end of stream.

@mschuwalow mschuwalow changed the title Expose end-of-stream from channels. Expose end-of-stream from channels Sep 6, 2020
@quelgar
Copy link
Contributor

quelgar commented Sep 6, 2020

This changed in #228, which is not in a release yet. The read method style is now def read(b: ByteBuffer): IO[IOException, Int], but in the case where we get -1 from the NIO read, the failure is an EOFException. So it is possible to differentiate the socket being closed, by catching EOFException, but you have to read the Scaladoc for read to know about it.

I also added an method eofOption you can use if you'd rather have the end-of-stream represented explicitly:

  /**
   * Turns `EOFException` failures into a success with no result.
   */
  def eofOption[R, A, E <: Throwable](effect: ZIO[R, E, A]): ZIO[R, E, Option[A]] =
    effect.asSome.catchSome {
      case _: EOFException => ZIO.none
    }

val maybeBytesRead: IO[IOException, Option[Int]] = eofOption(channel.read(buffer))

What do you think of this solution? Your version has the advantage that the end-of-stream is represented explicitly in the type, and will require explicit handling of some kind. This might be better overall, but I thought it might be annoying for code that doesn't need to specially handle end-of-stream.

I'd be happy to swap it around. Have read return IO[Option[IOException], Int], with a convenient way to turn that into IO[IOException, Int] where the None failure becomes an EOFException failure. Interested to hear opinions.

@mschuwalow
Copy link
Member Author

I'm happy with that solution 👍
It could still be a bit inconvenient if you compose it with other code that could return an EOFException, but much better than before.

For eofOption I would prefer to have Option in the error channel, as we can save some allocations that way.
This was also the reason for having the option in the error channel in ZStream.Pull iirc.

@quelgar
Copy link
Contributor

quelgar commented Sep 14, 2020

Yeah, using the error channel is a good idea, easy change.

quelgar added a commit that referenced this issue Sep 15, 2020
As discussed in issue #247.
@quelgar
Copy link
Contributor

quelgar commented Sep 15, 2020

#255 switches to using the error channel to indicate end-of-stream, and I added a note to the docs about how it works.

quelgar added a commit that referenced this issue Oct 21, 2020
As discussed in issue #247.
jczuchnowski added a commit that referenced this issue Oct 29, 2020
* Improve end-of-stream handling.

As discussed in issue #247.

* Update docs/essentials/index.md.

Improve wording.

Co-authored-by: Maxim Schuwalow <16665913+mschuwalow@users.noreply.github.com>

* Update for latest master.

Co-authored-by: Maxim Schuwalow <16665913+mschuwalow@users.noreply.github.com>
Co-authored-by: Jakub Czuchnowski <jakub.czuchnowski@gmail.com>
svroonland pushed a commit that referenced this issue Aug 9, 2021
* Improve end-of-stream handling.

As discussed in issue #247.

* Update docs/essentials/index.md.

Improve wording.

Co-authored-by: Maxim Schuwalow <16665913+mschuwalow@users.noreply.github.com>

* Update for latest master.

Co-authored-by: Maxim Schuwalow <16665913+mschuwalow@users.noreply.github.com>
Co-authored-by: Jakub Czuchnowski <jakub.czuchnowski@gmail.com>
quelgar added a commit that referenced this issue Sep 6, 2021
* Improve end-of-stream handling.

As discussed in issue #247.

* Update docs/essentials/index.md

Improve wording.

Co-authored-by: Maxim Schuwalow <16665913+mschuwalow@users.noreply.github.com>

* Switch to using ZManaged for core resources.

* Add resource management documentation.

* Fix extension method on Scala 3.

* Separate blocking and non-blocking APIs.

The channel APIs (except for asyncrhonous channels) are
now split into three parts:

* Blocking (BlockingOps)
* Non-blocking (NonBlockingOps)
* Core (On the channel itself)

Blocking and non-blocking APIs are the same in many cases
(GatheringByteOps and ScatteringByteOps), but there
are some differences.

The blocking API usage is performed as a block in
by using the `useBlocking` method on the channel.
This method switches to the ZIO blocking thread pool
and installs interrupt handling, which is necessary
to interrupt blocking I/O calls if the ZIO fiber is
interrupted. This imposes an overhead ever time it
is needed, this approach aims to pay the cost once
per channel, rather than once per read or write.

Non-blocking usage does not require the special
setup required for blocking, but for consistency
the API is accessed the same way, via the
`useNonBlocking` method on the channel.

* Improvements to WatchService.

* Add utility methods and example for directory watching.

* Improvements to Files.

* Use new ZStream adaptors for Java iterators and streams.
* Use the ZIO-NIO Charset wrapper
* Implement `lines` method
* Implement `copy` method

* Move Files object into core module.

* Add streaming read and write.

* Add example stream-based TCP server.

* Improve InetSocketAddress and socket binding APIs.

Use meaningful names for the InetSocketAddress
constructors.

Make binding to an automatically assigned socket
address explicit.

* Test that blocking read/write/accept can be interrupted.

* Remove non-core project.

* Rename nio-core project to nio.

* Rename zio.nio.core package to zio.nio.

* Make Selector#select interruptible.

* Fix SelectableChannel#register error type.

* Improve `SelectableChannel#register` API.

* Improve InetSocketAddress API.

* Don't hold two references to underlying Java buffer.

* Improvements to address handling.

* Add localhost constructor to InetSocketAddress.

* Improvements to async channels.

Make AsynchronousByteChannel callbacks interruptible by
closing the channel on interruption.

Looking at the JVM implementation, it seems async channel
callbacks only ever return IOException (not surprising)
so tighten the error types to IOException.

Add some socket channel write variants that were missing.

* Add stream/sink to AsynchronousFileChannel.

Co-authored-by: Lachlan O'Dea <lodea@mac.com>
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

3 participants