Propagate FS errors when creating a new project #10708
Conversation
radeusgd
added
the
CI: No changelog needed
radeusgd
requested review from
4e6,
JaroslavTulach,
hubertp and
Akirathan
as code owners
| pkg.save() | ||
| pkg.save().get |
There was a problem hiding this comment.
Discarding the result of Try[...]... - not the best idea
There was a problem hiding this comment.
What's the point of using Try there, then? .get will crash with an exception on failure and not really report the failure either.
| object CouldNotCreateDirectory extends Exception | ||
| case class CouldNotCreateDirectory(cause: Throwable) extends RuntimeException { | ||
| override def getMessage: String = | ||
| s"Could not create directory: ${cause.getMessage}. Perhaps there is a permission issue." |
There was a problem hiding this comment.
I added the comment, as for me this was caused by https://learn.microsoft.com/en-us/defender-endpoint/controlled-folders#what-is-controlled-folder-access
| System.err.println(ex.getMessage()); | ||
| ex.printStackTrace(); |
There was a problem hiding this comment.
getMessage is sometimes unhelpful for Java exceptions, e.g. NoSuchFileException::getMessage returns just the path - but no comment - so from getMessage you have no idea what happened, what kind of exception this was - you just see a file path and wonder where did this come from...
The stack trace should be more helpful I think.
There was a problem hiding this comment.
If so, the full stacktrace should be printed only when verbose mode is on. We don't want to show this to end users.
There was a problem hiding this comment.
What is the alternative? As mentioned, getMessage is utterly useless. I guess I could try toString.
| System.err.println(ex.getMessage()); | ||
| ex.printStackTrace(); |
There was a problem hiding this comment.
If so, the full stacktrace should be printed only when verbose mode is on. We don't want to show this to end users.
| pkg.save() | ||
| pkg.save().get |
There was a problem hiding this comment.
What's the point of using Try there, then? .get will crash with an exception on failure and not really report the failure either.
| @@ -1385,7 +1385,8 @@ private void launch(CommandLine line, Level logLevel, boolean logMasking) { | |||
| return BoxedUnit.UNIT; | |||
| }); | |||
| } catch (IOException ex) { | |||
| System.err.println(ex.getMessage()); | |||
| logger.error("Error during execution", ex); | |||
There was a problem hiding this comment.
You could check if (Logger.isEnabledForLevel(Level.DEBUG)) { ...
Pull Request Description
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.