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

Prevent native Error class hi-jacking in FS errors #40232

Open
SmashingQuasar opened this issue Sep 27, 2021 · 0 comments
Open

Prevent native Error class hi-jacking in FS errors #40232

SmashingQuasar opened this issue Sep 27, 2021 · 0 comments
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.

Comments

@SmashingQuasar
Copy link

Version

v16.9.0

Platform

Linux vagrant-docker 4.19.0-6-amd64 #1 SMP Debian 4.19.67-2+deb10u2 (2019-11-11) x86_64 GNU/Linux

Subsystem

fs

What steps will reproduce the bug?

In the current state of the promises section of the native fs module, when an error occurs, the native type Error is hi-jacked and a code property is being added to the error object.

This can easily be reproduced with the following code:

import { promises as FileSystem } from "fs";

try
{
    FileSystem.stat("/a/path/leading/to/nothing");
}
catch (error)
{
    console.log(Object.getPrototypeOf(error); // Will display the native class "Error"
    console.log(Object.getOwnPropertyDescriptors(error); // Will display the added code property that isn't native to Error.
}

How often does it reproduce? Is there a required condition?

No required conditions outside of the use of the fs native module.

What is the expected behavior?

This isn't a bug per-se but is a bad practice. A custom error of type FileSystemError should be thrown instead of hi-jacking the Error class to dynamically add a property to the object.
This behavior leads to problematic linting and testing with super-sets such as TypeScript or Eslint. Since code is not a native property of Error and dynamic property setting is often considered bad practice, it leads to situations where handling those specific issues requires extra effort to circumvent this design.

What do you see instead?

No response

Additional information

Ideally a custom FileSystemError should be created such as (Typescript example for types clarity):

class FileSystemError extends Error
{
    public code: string; // Ideally should be an enum listing all the possible values.
}
@targos targos added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

No branches or pull requests

2 participants