mafintosh / streamx

An iteration of the Node.js core streams with a series of improvements.
MIT License
226 stars 16 forks source link

Readable[Symbol.asyncIterator] does not signal done #55

Closed ThaUnknown closed 2 years ago

ThaUnknown commented 2 years ago

Readable[Symbol.asyncIterator] does not return an object of { done: true, value: null } when the stream finishes, instead it hangs the promise indefinitely, even the last successfully read iteration says the stream is not done

My code:

const { Readable } = require('streamx')

ReadableStream.prototype[Symbol.asyncIterator] = async function* () {
  const reader = this.getReader()
  let last = reader.read()
  while (1) {
    const temp = last
    last = reader.read()
    yield (await temp).value || null
  }
}

function BlobReadStream (blob, opts = {}){
  return Readable.from(blob.stream(), opts)
}
// ...
const stream = BlobReadStream(blob)
const asyncIterator = stream[Symbol.asyncIterator]()

let data
while (data){
  data = (await asyncIterator.next()).value
  console.log(data) // will never print null/undefined
}

the issue here seems to lay in passing null in Readable.from's iterator, but passing undefined isn't allowed in Node's readablestream spec, since it only allows Buffer, String or null

ThaUnknown commented 2 years ago

I think Readable.from(Symbol.asyncIterator) should end the stream with null if the data is null, undefined or iteration returns done, and Readable[Symbol.asyncIterator] should return { done: true, value: null } after the last value in the stream

mafintosh commented 2 years ago

I’m not sure what you mean. Can you share a simple test case I can run (in nodejs preferred) that illustrates the problem?

ThaUnknown commented 2 years ago

@mafintosh

const { Readable } = require('streamx')

async function test() {
  const obj = {
    [Symbol.asyncIterator]: async function* () {
      yield new Uint8Array([1, 2, 3, 4])
    }
  }
  const stream = Readable.from(obj)
  const iterator = stream[Symbol.asyncIterator]()
  let value = true
  while (value) {
    value = (await iterator.next()).value
    console.log(value)
  }
  console.log('this is never reached')
}

test().then(() => console.log('done'))

the while loop will never exit/finish, by extension, the function will never finish, this is because Readable[Symbol.asyncIterator] doesn't return null/undefined for its final value, and it also doesn't set done: true in the last iteration

for comparison: node returns undefined for its last value, with done: true node stream:

{ value: Uint8Array(4) [ 1, 2, 3, 4 ], done: false }
{ value: undefined, done: true }
this is never reached
done

streamx stream:

{ value: Uint8Array(4) [ 1, 2, 3, 4 ], done: false }

you can just replace streamx with stream in the require for testing

mafintosh commented 2 years ago

Thanks!

Could be boiled down to the following test case, https://github.com/streamxorg/streamx/blob/master/test/async-iterator.js#L174-L188

There was an uncaught in from that promises swallowed, but fixed in latest.

ThaUnknown commented 2 years ago

Thanks!

Could be boiled down to the following test case, https://github.com/streamxorg/streamx/blob/master/test/async-iterator.js#L174-L188

There was an uncaught in from that promises swallowed, but fixed in latest.

Great, happy to help, sorry I couldn't find the underlying issue, don't really understand the full structure of streams