sghpjuikit / player

Audio player and management application.
22 stars 2 forks source link

Error if properties file doesn't exist #165

Closed xeruf closed 5 years ago

xeruf commented 5 years ago

I got this error repeatedly over multiple starts. Sounds like it could be solved by a simple existance check.

14:39:23.158 [JavaFX Application Thread] ERROR sp.it.util.file.properties.Properties - Failed to read properties file= /home/janek/daten/projects/player/app/user/application.properties
java.io.FileNotFoundException: /home/janek/daten/projects/player/app/user/application.properties (No such file or directory)
        at java.base/java.io.FileInputStream.open0(Native Method)
        at java.base/java.io.FileInputStream.open(FileInputStream.java:219)
        at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157)
        at sp.it.util.file.properties.PropertiesKt.readProperties(Properties.kt:137)
        at sp.it.util.conf.Configuration.rawAdd(Configuration.kt:59)
        at sp.it.pl.main.App.<init>(App.kt:172)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
        at com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$8(LauncherImpl.java:802)
        at com.sun.javafx.application.PlatformImpl.lambda$runAndWait$12(PlatformImpl.java:455)
        at com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:428)
        at java.base/java.security.AccessController.doPrivileged(Native Method)
        at com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:427)
        at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
        at com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
        at com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(GtkApplication.java:277)
        at java.base/java.lang.Thread.run(Thread.java:834)
sghpjuikit commented 5 years ago

Thx for the report.

I'm assuming this is merely logging output, the application recovers and works as it should.

I'm using similar strategy elsewhere - avoid logging when catching FileNotFoundException.

xeruf commented 5 years ago

I'm assuming this is merely logging output, the application recovers and works as it should.

Yes

I'm using similar strategy elsewhere - avoid logging when catching FileNotFoundException.

It shouldn't be thrown in the first place by checking whether the file exists beforehand, because creating an Exception always carries overhead.

sghpjuikit commented 5 years ago

Doing the check would involve performance impact. APIs throwing exceptions work this way - pre-operation validation is handled by doing the actual operation and then handling the appropriate case. Imagine you are converting text to number, you just want to catch NumberFormatException without prior text checking to prevent the error. In that particular case it may make sense from performance perspective, but with IO, it is the opposite.

sghpjuikit commented 5 years ago

Ugh, the useLines... At first I thought Kotlin's shiny File extensions were great, but while useful, they do not take care of error handling and they do not even document which errors they throw!

sghpjuikit commented 5 years ago

Fixed in e7359bf

xeruf commented 5 years ago

I would recommend to at least print a debug log message when the FileNotFoundException is caught, it is not good for debugging when it happens silently.

sghpjuikit commented 5 years ago

We could, but I do not see much point. We do not provide comprehensive logging about what the application is doing and doing so would be non-trivial. Is there a reason why user/dev would need to make sure whether application has not found the properties file? Simple check to see if the file exists does the job.

xeruf commented 5 years ago

We do not provide comprehensive logging about what the application is doing

Then we should start, and it is best to start small. It has proven useful to me numerous times by now. Messages like this belong in debug, something that is rather spammy goes into trace. The user only sees info or even warn, so he won't care, but sometimes when debugging you might care. And it also gives you an indication of how far the app has loaded in case of crashes or smth.

sghpjuikit commented 5 years ago

Unfortunately, DEBUG logging is the least of my worries right now. Considering that I have 30h a week for this project, it would take like a week or two to get the logging up to satisfactory level. It's better to move in chunks for me for less context switching. Cutting concerns like these must be done consistently or else they add considerable overhead. Springling few logs here and there may just end up wasting time in the long run. Sorry.