[WasmFS] Make OPFS error handling more robust and regular#17774
Merged
[WasmFS] Make OPFS error handling more robust and regular#17774
Conversation
This was referenced Sep 1, 2022
Member
Author
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
kripken
approved these changes
Sep 1, 2022
| [&](auto ctx) { _wasmfs_opfs_open_blob(ctx.ctx, fileID, &id); }); | ||
| if (id < 0) { | ||
| return -EACCES; | ||
| return -id; |
Member
There was a problem hiding this comment.
Suggested change
| return -id; | |
| return id; |
kripken
reviewed
Sep 1, 2022
| // TODO: This is the correct error code for negative read positions, but we | ||
| // should handle other errors correctly as well. | ||
| return nread < 0 ? -EINVAL : nread; | ||
| return nread; |
Member
There was a problem hiding this comment.
I guess this means we can only read/write 2GB at a time? I suppose that's ok...
Member
Author
There was a problem hiding this comment.
In wasm32 the read syscalls can only read 2GB at a time anyway because their result type is a 32-bit ssize_t. We should update this to use the full 63 bits in wasm64, though. I think there's a TODO for that.
Member
There was a problem hiding this comment.
Ok, thanks. Doesn't sound urgent anyhow I guess, 31 bit reads is quite a lot...
kripken
approved these changes
Sep 1, 2022
Refactor the existing error handling in the OPFS backend according to these
principles:
1. Determine error codes where errors occur. This simplifies the code by
eliminating ad hoc magic error constants and improves the code by making
errors more specific.
2. Return errors in existing return channels where possible, rather than adding
new out-params for errors. This reduces code size and makes the code more
robust by reducing the space of meaningless return values.
3. Do not allow any exceptions to escape. Unknown exceptions are reported to
the console and converted to catch-all EIO error codes. Exceptions escaping
the async OFPS code cause deadlocks, so they must be rigorously avoided.
tlively
force-pushed
the
wasmfs-opfs-errors-refactor
branch
from
410395b to
f449ced
Compare
This was referenced Sep 1, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor the existing error handling in the OPFS backend according to these
principles:
Determine error codes where errors occur. This simplifies the code by
eliminating ad hoc magic error constants and improves the code by making
errors more specific.
Return errors in existing return channels where possible, rather than adding
new out-params for errors. This reduces code size and makes the code more
robust by reducing the space of meaningless return values.
Do not allow any exceptions to escape. Unknown exceptions are reported to
the console and converted to catch-all EIO error codes. Exceptions escaping
the async OFPS code cause deadlocks, so they must be rigorously avoided.