Handle del errors better

[tommy-mitchell]

If del-cli fails due to improper usage (e.g. deleting the cwd or a file outside the cwd), the error message is really noisy:

❯ del ~/some/dir
file:///.../del-cli/node_modules/del/index.js:37
                throw new Error('Cannot delete files/directories outside the current working directory. Can be overridden with the `force` option.');
                      ^

Error: Cannot delete files/directories outside the current working directory. Can be overridden with the `force` option.
    at safeCheck (file:///.../del-cli/node_modules/del/index.js:37:9)
    at mapper (file:///.../del-cli/node_modules/del/index.js:83:4)
    at file:///.../del-cli/node_modules/p-map/index.js:141:26
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Node.js v18.17.1

These errors should be caught and console.errored instead:

❯ del ~/some/dir
Cannot delete files/directories outside the current working directory. Can be overridden with the `force` option.

Maybe del could export a DelError class, and del-cli could catch only those errors.

[sindresorhus]

I actually have been wanting to make a package for this use-case. It's a common need for a package to have some errors that should be presented to the user without a stack trace. The difficulty is to separate these types of errors. Making a custom error class each time is one possibility. But convention would be ideal.

What do you think of something like this?

class PresentableError extends Error {
	constructor(message) {
		super(message);
		this.name = 'PresentableError';
		this.message = message;
	}
}

try {
	throw new PresentableError("Something went wrong, please try again.");
} catch (error) {
	if (error instanceof PresentableError) {
		console.log(error.message); // Output: Something went wrong, please try again.
	} else {
		throw error;
	}
}

We could have a utility that wraps a throwable call and does the correct handling.

Some other names that could work:

• DisplayableError
• HumanFriendlyError
• SoftError

The downside of having a subclass like this is that the consumer has to import this package to be able to identify presentable errors.

An alternative approach is to also define a unique error property as convention. For example: Error#isPresentableError. This way it could be added to any error without needing to subclass PresentableError (instanceof also does not work across realms).

[tommy-mitchell]

I like the idea. I also like how SoftError kind of matches with hard/loud-rejection.

Would this be used in the CLI/etc. or in the library itself? If the former, how would it distinguish which errors to display softly (like the del-specific errors)?

[sindresorhus]

Would this be used in the CLI/etc. or in the library itself? If the former, how would it distinguish which errors to display softly (like the del-specific errors)?

It would be used by the library itself. Say a CLI uses multiple packages that may potentially throw, it would be good to have this convention instead of having to handle custom errors for each package separately.

[sindresorhus]

I will do a prototype.

[sindresorhus]

I like the idea. I also like how SoftError kind of matches with hard/loud-rejection.

SoftError may be too ambigious as it does not hint that it's an error that will be presented to the user. I'm leaning towards PresentableError or DisplayableError, and mostly the former.

The name is not as important as the property though. It needs to be something that could be added to any error without risk of conflict.

[tommy-mitchell]

Maybe meow could export an

isPresentableError(error: unknown): error is PresentableError

so CLIs get type safety without the need to list the package as a dependency:

import meow, {isPresentableError} from "meow";

try {
  someThrowableDependency();
} catch (error) {
  if (isPresentableError(error)) {
    console.error(error.message);
    process.exit(1);
  }
  
  throw error;
}

Or maybe even a throwIfNotPresentable(error) that encapsulates the entire catch block.

[sindresorhus]

The idea is that you should be able to make any error presentable, so we could say that as long as the error is a Error instance and contains a Error#isPresentable property, it's presentable. That also makes it easy to check.

[sindresorhus]

Continued in sindresorhus/presentable-error#2.

[sindresorhus]

Prototype done: https://github.com/sindresorhus/presentable-error (Feedback wanted)

[fregante]

I think that generally no error should display the stack trace to the user, so every error should be presented "beautifully".

So what about wanting to see the stacktrace during debugging/development? Well… DEBUG=1 del some-file, or any other way that the app defines (in this case del-cli).

import process from 'node:process';
import del from 'del';

function handleError(error) {
	if (process.env.DEBUG) {
		throw err;
	} else {
		console.error(error.message);
		process.exitCode = 1;
	}
}

process.on('uncaughtException', handleError);
process.on('unhandledRejection', handleError);

del(process.argv[2]);

Also mentioned in https://twitter.com/fregante/status/1716279058034733187, I don’t think "presentability" is up to the error producer.

I'd say that if you want to standardize error presentability, you could publish the above code as an "error boundary" general-use package so that this logic and ENV can be shared and refined further.

import 'cli-error-boundary';
import del from 'del';

del(process.argv[2]);

But presumably in your case this can just be part of meow

[sindresorhus]

I think that generally no error should display the stack trace to the user, so every error should be presented "beautifully".

Define "users". End-users (of an app, for example) or developers (users of a package)?

And the problem with never presenting stack trace is that many errors cannot be reproduced easily. So you cannot simply run it again with DEBUG=1 to get stack trace. I learned that the hard way with np.

This is why I want to separate expected user-friendly errors from unexpected errors. Expected errors could be invalid input. Unexpected errors could be trying to access an undefined property somewhere.

I'd say that if you want to standardize error presentability, you could publish the above code as an "error boundary" general-use package so that this logic and ENV can be shared and refined further.

Yes, that's indeed something I have thought of, but it's a little bit early to optimize the code.