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

Handle errors from fileType.stream method #195

Merged
merged 2 commits into from
Apr 9, 2019

Conversation

chytreg
Copy link
Contributor

@chytreg chytreg commented Apr 2, 2019

There are cases like an empty stream then fileType(chunk) will throw an error. The error is not propagated correctly and cannot be caught in the main plot. Using reject solves the problem and allows to handle error in the main plot of the app.

try {
  fileTypeStream = await fileType.stream(stream);
} catch (error) {
  // without reject, you won't catch the error thrown by fileType(check) here
  console.log(error);
}

There are cases like an empty stream then fileType(chunk) with throw an error. The error is not propagated correctly and cannot be caught in the main plot. Using reject solves the problem and allows to handle are in the main plot of the app.

try {
  fileTypeStream = await fileType.stream(stream);
} catch (error) {
  // without reject, you won't catch the error thrown by fileType(check) here
  console.log(error);
}
@sindresorhus
Copy link
Owner

@bencmbrook Thoughts?

@bencmbrook
Copy link
Contributor

@sindresorhus this is a solid PR. I tested it independently and this does improve the stack trace. It doesn't change the API per se, as it will still throw an error in a similar manner, but if it's uncaught at the top-level, this PR will make it easier to debug.

@chytreg might consider adding a more verbose error describing the empty stream.

@chytreg
Copy link
Contributor Author

chytreg commented Apr 5, 2019

@bencmbrook I think we may try to improve a message for TypeError of fileType method.

const fileType = input => {
	if (!(input instanceof Uint8Array || Buffer.isBuffer(input))) {
		throw new TypeError(`Expected the \`input\` argument to be of type \`Uint8Array\` or \`Buffer\`, got \`${typeof input}\``);
	}

typeof(null) => "object",
typeof(undefined) => "undefined"

const fileType = input => {
	if (!(input instanceof Uint8Array || Buffer.isBuffer(input))) {
                const inputTypeMsg = (input) ? typeof(input) : "an empty input";
		throw new TypeError(`Expected the \`input\` argument to be of type \`Uint8Array\` or \`Buffer\`, got \`${inputTypeMsg}\``);
	}

@chytreg
Copy link
Contributor Author

chytreg commented Apr 5, 2019

We can handle null also as follow:

const fileType = input => {
        if (!input) { return null }
	if (!(input instanceof Uint8Array || Buffer.isBuffer(input))) {
                const inputTypeMsg = (input) ? typeof(input) : "an empty input";
		throw new TypeError(`Expected the \`input\` argument to be of type \`Uint8Array\` or \`Buffer\`, got \`${inputTypeMsg}\``);
	}

@bencmbrook
Copy link
Contributor

bencmbrook commented Apr 7, 2019

@chytreg isn't this essentially what would get thrown anyway after it tries to read a chunk?

@sindresorhus sindresorhus changed the title Handle errors from fileType.stream method Handle errors from fileType.stream method Apr 9, 2019
@sindresorhus sindresorhus merged commit 7bbbacc into sindresorhus:master Apr 9, 2019
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