WebAssemblyOS / wasmos

OS primitives and shell for AssembyScript and WebAssembly
MIT License
42 stars 21 forks source link

Unsure of how error handling works #95

Open saltwick opened 5 years ago

saltwick commented 5 years ago

I'm working on an error command (#49) to report if the last command failed, but I'm unsure of how commands report their errors. I've looked through the Wasi, WasiResult, and Console types, but can't find how commands report their exit status.

Should I be changing the Console.error() function to set a flag that I can then check in the error command? Or should I not be changing the types that are already there?

willemneal commented 5 years ago

So this one is actually not a real Unix command, rather one that shell.js uses.

function error() {
  return common.state.error;
}

I made a mistake in the beginning by having my sample programs' main functions return void instead of an integer to signify an error. Also my current implementation of Process.exit calls an abort function causing the whole binary to exit. However, in the scope of this assignment, I think a fair assumption is that all programs will share Console.error and that anything written to it constitutes that an error has occurred. For example,

cat(["cat", "/filethatdoesntexist"])

will write to stderr. Console has a file descriptor for it you can access with Console.stderr. Thus if this file descriptor has a non-zero offset, you can assume that something was written to it and an error has occurred.

This program is hard because we don't have a shell yet so invoking programs is just a function call. Otherwise we could add state to the shell that you could read to determine if the last run command had an error.

saltwick commented 5 years ago

I found that checking if Console.stderr.tell() > 0 would work as it is how the current tests check to see if an error occurred. Beyond that, since we don't have a shell like you said, is there nothing else that can be done for this?

willemneal commented 5 years ago

I mean you could edit Process.exit and add a static error variable. This way people can write to stderr with actually having an error, which is useful if you want to write warnings but not exit with an error.

Consider cat foo README.md || echo "no error". Even though foo doesn't exist, it prints that error to stderr and then prints README.md to stdout. And returns "false" meaning there was an error and so the other side of the || is executed and no error is written to stdout. Since people expect Process.exit() to actually exit there program would stop after foo and not read in the second argument. Thus for now, I think a non empty stderr is the best work around.

saltwick commented 5 years ago

I see that Process has an exit function in it's type definition, but I can't find it anywhere in kernel/src/process. Where is the exit function?

willemneal commented 5 years ago

Ah yes, everything in a src directory is typescript. It's in assemblyscript/assembly/wasa/mock/process.ts. Perhaps you could add a function separate from exit for this purpose since like I said before users will assume that exit means exit and could use that to say that an error has occurred, just not bad enough to exit the program completely.

saltwick commented 5 years ago

I think I've figured it out, but I'm having trouble accessing Process from my error.ts file in the ash package. When I try to import process from assembly script, it keeps telling me that the module cannot be found.

willemneal commented 5 years ago

Hmm not sure exactly what you mean. If you push to your PR, then I can take a look. The idea with Process, Console, etc, is that you shouldn't have to import them in your binary program, rather they are globals that are provided by the runtime. This allows them to be swapped out, currently they are mocks, but I'm working making them actually call out to Wasi.

If you did add a function to Process you should edit the types/wasi/index.d.ts file which currently declares Process like so,

/**
 * Provides function to exit.
 */
declare class Process {
  static exit(code: number): void;
}

Then your editor will know that there exists a Process defined somewhere that will have that function. Then the testing framework already includes the Process implemented by the mocks. So it will be linked at compile time. Make sense?

saltwick commented 5 years ago

That makes sense, I see none of the example commands import anything. Does the same go for Wasi? If I wanted to use the Wasi.errno enums, should they be accessible from these functions?

willemneal commented 5 years ago

I don't think so since Wasi is a namespace and I could only get it to work with classes. However, importing Wasi should work.

saltwick commented 5 years ago

That also makes sense. My current issue is that my tests are failing, even though they said Expected and Actual are both 1. I'm getting a RuntimeError (unreachable) as well, but I'm not sure how to interpret the message given. I've pushed the code up, can you take look?

EDIT: Nevermind with this, I was calling echo wrong.

saltwick commented 5 years ago

To my knowledge it should be working correctly, but my Process.error() is not working properly.

I set it up so that a command like cat would call Process.error(Code) if it raises an error, which would then set the static flag to that code. Then in the ash error() command it checks if the flag is not 0 (SUCCESS). However the flag is never changing when I call Process.error, but I'm not sure what's wrong there.

willemneal commented 5 years ago

So when I ran your tests I got:

    [Stack]: RuntimeError: unreachable
               at ~lib/array/Array<~lib/string/String>#__get (wasm-function[68]:50)
               at packages/ash/assembly/bin/echo/main (wasm-function[257]:23)
               at start:packages/ash/assembly/__tests__/error.spec~anonymous|0~anonymous|2 (wasm-function[258]:12)

This means that you tried to get something from the array and it was out of bounds. I looked at your code it's because you didn't add the command name to the command line arguments. Silly I know by it's tradition.

willemneal commented 5 years ago

Well test does exist ;-) checkout __test__/simple_fs.ts

saltwick commented 5 years ago

I think I'm confused as to how process works. I created a static variable that gets set with a call to Process.error(). I changed cat to call Process.error() when it errors, but either that never changes the static variable's value, or I am not properly retrieving that variable in my error command.

willemneal commented 5 years ago

You're assumptions are correct and it seems correct. Have you changed the test to something else? Otherwise it shouldn't have an error as test does exist.

saltwick commented 5 years ago

I see, that fixed it! Now I need to figure out how to reset the flag back. Since technically when you use the error ash command, that is your new last command, it would be alright to reset the error flag when error is called from ash, right?

saltwick commented 5 years ago

I got it working and my tests pass. When there is an error, 1 (truthy-enough?) is returned. If there wasn't an error on the last command, then 0 is returned. Thanks for the help!

willemneal commented 5 years ago

I think it's a safe assumption that ash will do this when it executes a command. Sounds good!