Skip to content

Unified way to clean up after ReadableStream finished/errored/was canceled #1329

Open
@jedwards1211

Description

@jedwards1211

What is the issue with the Streams Standard?

It seems like the standard doesn't provide a simple way to guarantee the resources underlying a ReadableStream are cleaned up after it ends for any reason. There are up to four cases we have to take care of:

  • start throws an error or rejects
  • pull throws an error or rejects
  • stream is canceled
  • stream ends naturally and closes itself

I mean, I could make a shared cleanup function, but it's still up to me to make sure I call it in each of those four cases:

async function cleanup() {
  // free resources
}

return new ReadableStream({
  async start(controller) {
    try {
      ...
    } catch (error) {
      await cleanup()
    }
  },
  async pull(controller) {
    let needsCleanup = false
    try {
      const next = ...
      if (next) {
        controller.enqueue(next)
      } else {
        controller.close()
        needsCleanup = true
      }
    } catch (error) {
      needsCleanup = true
      throw error
    } finally {
      if (needsCleanup) await cleanup()
    }
  },
  async cancel(controller) {
    await cleanup()
  }
})

I think it should also accept a closed method on underlyingSource that will be called after the stream has closed for any reason:

return new ReadableStream({
  async start(controller) {
    ...
  },
  async pull(controller) {
    ...
  },
  async closed() {
    // cleanup code
  },
})

We can do something like this in userland, but if we leave it up to userland, there's a lot more risk that developers will miss one of the four cases where they need to cleanup. If the API supports a closed method itself, then there will be fewer resource leaks in the wild.

Also, I think most programmers wouldn't want to duplicate the above boilerplate for every stream that needs to clean up resources, so they would end up using a userland shim that supports a closed handler instead of using ReadableStream directly. Well then, to avoid copying that shim between projects, someone would make a readablestream-closed npm package etc, and eventually we'd have tons of projects depending on this little npm package just to write ReadableStreams safely.

I don't think we want a future where the best practice answer to "how do I make a custom ReadableStream?" is "do it via this package, it helps you avoid resource leaks", and a lot of people who aren't aware of that have accidental resource leaks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions