search

eclipse-archived / californium.core

Californium project
86 stars 69 forks source link

Replaced printStackTrace calls to LOGGER.log calls in CoapEndpoint an… #48

Closed eugene-nikolaev closed 8 years ago

eugene-nikolaev commented 8 years ago

Hello! Could you please consider this pull-request?

My app is connected to a error-tracking service - rollbar.com Rollbar's library is implemented as appender to Logback. I've forwarded JUL logs to logback, it is not difficult. But I've found that if my app code was inside CoapHandler#onLoad and it thrown some RuntimeException, the exception just was forwarded to stdout and rollbar could not get it. Sure, now I have try/catched my handlers, but I wish also Californium's unhandled exceptions be forwarded to the rollbar too. Is there any reason to have such printStackTrace calls?

Regards, Eugene.

mkovatsc commented 8 years ago

The reason is that these cases occur out of the scope of the framework and the stack trace is very helpful when debugging the user application (just like the JVM would handle such user code). However, it makes good sense to log them as well!

There are also some other occurrences and for some both logging and the output is useful. I think I will fix this accordingly and add describing log messages.

@sophokles73 can you quickly comment on the printStackTrace() in Base64? A severe log entry should suffice, right?

eugene-nikolaev commented 8 years ago

Thank you! Sure, printStackTrace is better than nothing. I've meant was there any reason to keep them instead logging..

mkovatsc commented 8 years ago

I commited the fixes in 9012938ffa5cd52473640b0438ffba7dd3adc769. I am leaving this open until Kai comments on the print in Scandium.

eugene-nikolaev commented 8 years ago

BTW, there are two more calls to printStackTrace in ProxyCacheResource at californium-proxy

sophokles73 commented 8 years ago

@mkovatsc

are you referring to the Base64 class in the Scandium code? If so, I do not think that we need any stack traces being printed to standard error or out ... so, from my point of view these should be replaced with a log statement. Depending on the kind of problem occurring, a Level.INFO or Level.FINE should also suffice since we are usually not talking about cases where the system does not work properly anymore but a somewhat expected exception occurred but did not cause any particular harm.

sophokles73 commented 8 years ago

I have replaced the call to printStackTrace with a log statement in Scandium.

@mkovatsc

I am pretty sure that if you use something like

} catch (Exception e) {
   Logger.log(Level.INFO, e.getMessage(), e);
}

this will include a stack trace in the log statement.

mkovatsc commented 8 years ago

Okay, closing this. All occurrences were fixed. Thanks for the feedback!