Skip to content

Conversation

@avoylenko
Copy link

Do not throw the EndOfStreamError() exception in case you pass only partial zip data.

Code to reproduce the error:

file.stream.once('data', async (firstChunk) => {
    const type = await fileTypeFromBuffer(firstChunk)
})

Stack trace:

EndOfStreamError: End-Of-Stream
at BufferTokenizer.peekBuffer (file:///opt/novatalks.engine/node_modules/strtok3/lib/BufferTokenizer.js:38:19)
at BufferTokenizer.readBuffer (file:///opt/novatalks.engine/node_modules/strtok3/lib/BufferTokenizer.js:24:38)
at BufferTokenizer.readToken (file:///opt/novatalks.engine/node_modules/strtok3/lib/AbstractTokenizer.js:32:32)
at ZipHandler.unzip (file:///opt/novatalks.engine/node_modules/@tokenizer/inflate/lib/index.js:127:61)
at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
at async Object.detectConfident [as detect] (file:///opt/novatalks.engine/node_modules/file-type/core.js:521:4)
at async FileTypeParser.fromTokenizer (file:///opt/novatalks.engine/node_modules/file-type/core.js:211:21)

@sindresorhus
Copy link
Owner

Needs a test. And I'm not sure it's correct to just completely silence it.

@avoylenko
Copy link
Author

@sindresorhus do you want me to push a test that covers this scenario?

@sindresorhus
Copy link
Owner

Yes

@avoylenko
Copy link
Author

avoylenko commented Aug 6, 2025

Just pushed the test, hope it suits the test structure.

core.js Outdated
return {};
}
});
}).catch(() => {});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to check if the error caught is an EndOfStreamError error, if not throw the error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented as required

Copy link
Collaborator

@Borewit Borewit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

3 participants