Skip to content
This repository has been archived by the owner on Apr 13, 2024. It is now read-only.

Convert filesystem API errors to a consistent type #46

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

AtkinsSJ
Copy link
Contributor

@AtkinsSJ AtkinsSJ commented Mar 5, 2024

The Puter and Node filesystem APIs now convert most errors to a new PosixError class, which holds a standard POSIXy error code. PosixError can be constructed manually, or by using one of the PosixError.Foo() helper functions which produce a default error message.

The Puter and Node filesystem APIs now convert most errors to a new
PosixError class, which holds a standard POSIXy error code. PosixError
can be constructed manually, or by using one of the PosixError.Foo()
helper functions which produce a default error message.
if (ErrorCodes[Symbol.keyFor(posixErrorCode)] !== posixErrorCode) {
throw new Error(`Unrecognized POSIX error code: '${posixErrorCode}'`);
}
this.posixCode = posixErrorCode;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default for error classes seems to be .code for the error code, would that make more sense? I wasn't sure whether to make it distinct or not.

Comment on lines +19 to +37
export const ErrorCodes = {
EACCES: Symbol.for('EACCES'),
EADDRINUSE: Symbol.for('EADDRINUSE'),
ECONNREFUSED: Symbol.for('ECONNREFUSED'),
ECONNRESET: Symbol.for('ECONNRESET'),
EEXIST: Symbol.for('EEXIST'),
EFBIG: Symbol.for('EFBIG'),
EINVAL: Symbol.for('EINVAL'),
EIO: Symbol.for('EIO'),
EISDIR: Symbol.for('EISDIR'),
EMFILE: Symbol.for('EMFILE'),
ENOENT: Symbol.for('ENOENT'),
ENOSPC: Symbol.for('ENOSPC'),
ENOTDIR: Symbol.for('ENOTDIR'),
ENOTEMPTY: Symbol.for('ENOTEMPTY'),
EPERM: Symbol.for('EPERM'),
EPIPE: Symbol.for('EPIPE'),
ETIMEDOUT: Symbol.for('ETIMEDOUT'),
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using Symbols is nice, but also means that there's a small trap when comparing the error code. If you try if (e.posixCode === 'ENOENT') it'll fail. But this might not be an issue in practice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The same symbol object needs to be attained, for example e.posixCode === ErrorCodes.ENOENT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. My thinking is this becomes a trap for users because it'd be easy to forget to do that. But then, JS lets you compare it against literally anything so maybe people will be used to that, and it's not an issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, somewhere the type of posixCode should be documented but then once all the other comparisons are correct contributors will use those as a reference anyway

case 'batch_missing_file': return new PosixError(ErrorCodes.EINVAL, e.message);

// Open
case 'no_suitable_app': break;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These non-file-related ones are left here for reference. We might want to convert them properly later. As is, they'll just be returned without being modified.

@AtkinsSJ
Copy link
Contributor Author

AtkinsSJ commented Mar 5, 2024

An extra thought about this: For errno, and returning specific errors as exit codes, they'll need integer values. That will require some tweaking here.

@KernelDeimos KernelDeimos merged commit 9a1d5f7 into HeyPuter:trunk Mar 6, 2024
2 checks passed
@AtkinsSJ AtkinsSJ deleted the filesystem-consistent-errors branch March 6, 2024 17:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants