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

Missing async support for Reader._createReadStream() #17

Open
dvpritzbuer opened this issue Jun 9, 2023 · 2 comments
Open

Missing async support for Reader._createReadStream() #17

dvpritzbuer opened this issue Jun 9, 2023 · 2 comments

Comments

@dvpritzbuer
Copy link

dvpritzbuer commented Jun 9, 2023

Hi,

I use yauzl-promise v4.0.0 and want to use the yauzl.fromReader() method in combination with azure's blob storage client and have created a custom reader class with the following code:

import { BlobClient } from "@azure/storage-blob";
import * as yauzl from "yauzl-promise";

export class MyReader extends yauzl.Reader {
  constructor(private readonly blobClient: BlobClient) {
    super();
  }

  async _createReadStream(start: number, length: number) {
    const response = await this.blobClient.download(start, length);
    if (!response.readableStreamBody) throw new Error("no stream");
    
    return response.readableStreamBody;
  }

  async _read(start: number, length: number) {
    const buffer = await this.blobClient.downloadToBuffer(start, length);
    
    return buffer;
  }

As you can see in this example, I'm not able to provide a _createReadStream implementation without this function to be async and await the blobClient download.

But then I always get the following exception:
image

Maybe I use the _createReadStream wrong and there is a way for me to provide this method without being async, but if the yauzl.Reader class would await the _createReadStream function, it would solve my problems.

Every part(at least I need) that uses the _createReadStream function is async anyways and I just had to update the following lines to get my custom reader working:

  1. entry.js (add await)
    image

  2. reader.js (add await)
    image

  3. reader.js (add async)
    image

Thanks for your help!

@overlookmotel
Copy link
Owner

Yes, Reader.prototype._createReadStream is not async. It expects the ReadStream to be created synchronously.

I can see that your use case would be easier if _createReadStream() was async, and probably that should be supported, though it'd be a breaking change.

I am crazy busy with work for next month, so won't have time to look at this until July (or later). I'd welcome a PR (with tests), and that would likely get it implemented earlier than it would otherwise.

You can also achieve what you want with the current API with something like this:

import {PassThrough, pipeline} from 'node:stream';

export class MyReader extends yauzl.Reader {
  _createReadStream(start, length) {
    const passThroughStream = new PassThrough();

    (async () => {
      try {
        const response = await this.blobClient.download(start, length);
        if (!response.readableStreamBody) throw new Error("no stream");
        // No need for pipeline callback to do anything.
        // Errors will be emitted on passThroughStream anyway.
        pipeline([response.readableStreamBody, passThroughStream], () => {});
      } catch (err) {
        passThroughStream.emit('error', err);
      }
    })();

    return passThroughStream;
  }

  // Other methods omitted in this example
}

That example might not cover all bases as far as error handling is concerned, but hopefully you get the idea.

@dvpritzbuer
Copy link
Author

Thanks for your quick response.

I tried your workaround with the passThrough stream, but then I get following error: "TypeError: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of PassThrough".

I'm fine with opening a PR and support you as much as I can to get this topic published.
I'm not sure about the tests and documentation updates, but I guess we can clarify these within the PR based on my changes.

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