gaz561 / tab

Groundhog Operating System
1 stars 1 forks source link

tab.scan-directory should error if the directory ain't found #4

Open emsenn opened 2 years ago

emsenn commented 2 years ago

I tried some different things with assert and stuff but I think I'm just not understanding the relationships going on between the host system and TAB when using scan-directory.

But basically if find complains the directory ain't found, scan-directory should return an error.

(Can we also talk about what returning an error should look like? Is it returning two values, the first nil and the second the error message? and then using match to catch them? Because that's what it seems like in different projects but I'm not sure and so have been avoiding any error handling which is NOT the right solution.)

Just gonna go ahead and tag @technomancy <3

technomancy commented 2 years ago

Can we also talk about what returning an error should look like?

Oh man, great question. So as you know many functions use the style of returning nil along with a message for errors, (in Fennel this is done with (values nil "problem reading the directory")) while others use the assert and error functions. It's very easy to turn nil/msg into an error (using assert) and it's easy to turn an error into nil/msg (using pcall) so it comes down to what makes the most sense in the context.

The advantage of assert/error is that it works "deeply"; that is, if you have a very deep call stack and want to return control to many layers up, the error will punch all the way thru, either terminating the program or jumping to the most recent pcall. Sometimes this can be really convenient! But if you anticipate errors and want to handle them directly, IMO it's usually nicer to use the other style. Fennel has a new-ish macro called match-try which lets you declare a series of steps (kind of like ->) where each step is matched against a pattern, and if it doesn't match, it falls down to a catch clause.

(fn handle [conn]
  (match-try (conn:receive :*l)
    input (parse input)
    (command-name params) (commands.get command-name)
    command (pcall command (table.unpack params))
    (catch
     (_ :timeout) nil
     (_ :closed) (pcall disconnect conn "connection closed")
     (_ msg) (print "Error handling input" msg))))

I've found if you don't need the "deep" property of error that this can lead to some really nice declarative code! But if you don't have a series of steps then match will do just as well.

So that's my thoughts on errors in general. On the other hand specifically for this directory not being found in scan-directory, at a glance it looks like this kind of error should probably be fatal and not something you necessarily try to code around. Like ... if the directory is missing, it doesn't really make sense to keep running the program, right, because it can't do anything useful without that data? So for that I'd just use an assert. The scan-directory function isn't currently calling (results:close) to close the find command, which it should probably do. IIRC the close call is the thing that gives you the exit code, so (assert (results:close) "could not find directory") ought to do the trick here.

Hope that helps.