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

classlib: File.mkdir should fail on failure #3635

Closed
mossheim opened this issue Apr 5, 2018 · 8 comments
Closed

classlib: File.mkdir should fail on failure #3635

mossheim opened this issue Apr 5, 2018 · 8 comments
Labels
API change comp: class library SC class library comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Milestone

Comments

@mossheim
Copy link
Contributor

mossheim commented Apr 5, 2018

As pointed out in #3633, File:*mkdir just prints a warning if it fails to complete. This makes checking for failure impossible. It should instead fail in some helpful way, like internally throwing an exception or returning a nonzero error code.

@mossheim mossheim added this to the future milestone Apr 5, 2018
@jamshark70
Copy link
Contributor

Yeah, I just ran into this myself.

Probably all of the file primitives should be checked. It looks like they assume everything is just going to work (a quite remarkable assumption for software engineers to make).

I think I'll add sclang as a component, since the primitive will have to return the error code.

@jamshark70 jamshark70 added the comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library" label Jun 16, 2018
@mossheim
Copy link
Contributor Author

It looks like they assume everything is just going to work (a quite remarkable assumption for software engineers to make).

Remember that many of the boost.filesystem calls in that code are the throwing versions. The wrapping primitive doesn't need to return an error code in this case.

@jamshark70
Copy link
Contributor

OK, quick survey:

  • File.copy("nonexistent", "nonexistent2"); -- throws error (OK)

  • File.delete("nonexistent"); -- returns true (I guess it's reasonable -- the objective is to be sure the file doesn't exist after the operation, and this is also true if the file never existed)

  • File.delete("/usr/local/bin/sclang"); -- fails because of insufficient permissions, and returns false -- OK, there's a way to detect failure

  • File.fileSize("nonexistent"); -- throws error (OK)

  • File.mtime("nonexistent"); -- throws error (OK)

  • File.type("nonexistent"); -- throws error (OK)

So (unless I missed any methods), it seems mkdir is the only one that doesn't report failure.

@jamshark70
Copy link
Contributor

FWIW, yesterday I lost 6 of 25-30 student recordings (about 20% failure rate) because, despite adding File.mkdir(Platform.recordingsDir) into my code, there's no reliable way to detect failure and warn the user.

If someone can point me to the error-throwing version of boost mkdir, I'll be happy to submit a PR.

IMO this shouldn't be "future" -- it should be 3.10. Lost data from a live performance is not acceptable.

@mossheim
Copy link
Contributor Author

@jamshark70 it's easy to find the reference documentation for boost.filesystem with a quick google search. Throwing behavior is clearly defined there

@joshpar
Copy link
Member

joshpar commented Jun 23, 2018 via email

@jamshark70
Copy link
Contributor

jamshark70 commented Jun 25, 2018

Thanks @joshpar, I'll try to make a PR today.

@brianlheim It was a really busy week last week, like, come home and collapse on the sofa busy. It's awkward, a lot of open issues that I have a stake in, and I'm tired... really tired. I apologize that I didn't do my homework but I can't keep up with everything.

@mossheim
Copy link
Contributor Author

mossheim commented Jul 2, 2018

Closed in #3813. Thanks for the PR @jamshark70 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change comp: class library SC class library comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Projects
None yet
Development

No branches or pull requests

3 participants