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

Allow async "wrapper" callbacks #234

Open
MannikJ opened this issue Mar 23, 2023 · 3 comments
Open

Allow async "wrapper" callbacks #234

MannikJ opened this issue Mar 23, 2023 · 3 comments

Comments

@MannikJ
Copy link

MannikJ commented Mar 23, 2023

Please see this codesandbox:
(link removed)

Important part (shortened):

  return this.newParser()
     .namely("data")
     .endianess("little")
     .wrapped("data", {
       readUntil: "eof",
       wrapper: async (originalByteArray) => {
         try {
           const lzma = new LZMA();
           const result = await lzma.decompress(newByteArray);
           return Buffer.from(result.buffer);
         } catch (error) {
           console.error(error);
         }
       },
       // The parser to run on the decompressed data
       type: decompressedDataParser
     });

It should parse some custom file format which contains a LZMA-compressed portion which I would like to decompress within the wrapper function. The parser is not fully implemented, but it fails somewhere In the wrapping with

TypeError
imports[1].call(...).subarray is not a function

I've tinkered so much now, but I still can't get rid of this strange error and I don't know how to debug further. I thought it comes from a wrong return type of my wrapper function, but I am quite sure that this is not the problem. No matter what I return from the wrapper function, the error always occurs. It returns the same error message when I skip the whole processing and only return a new Buffer like this: return Buffer.from(new Uint8Array(32));
When I only use buffer instead of wrapped the parsing process runs through. So there is definitely some problem with the wrapping part.

I really hope you can help me with that!

@MannikJ
Copy link
Author

MannikJ commented Mar 28, 2023

It really took me way too long to realize that this error comes from the async nature of the callback. I added a shortened code snippet to the issue description, so there is no need anymore to go into the codesandbox project to understand the problem.

What happens is that the library calls the passed callback like expected, but if it is an async function, it just continues executing with an unresolved Promise and therefor throws this error.

In fact the documentation does not mention this restriction explicitly, but implies it by using the synchronous version of the decompress function in the example:

const mainParser = Parser.start()
  // Read length of the data to wrap
  .uint32le("length")
  // Read wrapped data
  .wrapped("wrappedData", {
    // Indicate how much data to read, like buffer()
    length: "length",
    // Define function to pre-process the data buffer
    wrapper: function (buffer) {
      // E.g. decompress data and return it for further parsing
      return zlib.inflateRawSync(buffer);
    },
    // The parser to run on the decompressed data
    type: textParser,
  });

@Rzial @keichi Is there anything we can do to make this work? I would also try to prepare a pull request, but I'd probably need some help, because after the first look the code does not appear self-explanatory to me and I need to figure out how to set it up first.

@keichi
Copy link
Owner

keichi commented Mar 30, 2023

Hi @MannikJ, not supporting async parsing was an early design choice, and it would require a significant redesign to support it.

Maybe you can parse in two passes? The first parser would extract the compressed data and the second parser would parse the decompressed data.

@MannikJ MannikJ changed the title Problem Using "wrapped" Functionality Allow async "wrapper" callbacks Mar 30, 2023
@MannikJ
Copy link
Author

MannikJ commented Mar 30, 2023

Thank you for the answer! I thought it's not such a simple change and already implemented the suggested workaround.
It would still be very cool if the library could support that feature some time.

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

No branches or pull requests

2 participants