Repository navigation
add log for catch (Exception) clause - #68
Andy Xu(devdiv) (andxu) wants to merge 17 commits into
Conversation
| } | ||
|
|
||
| if (!(container instanceof VariableProxy)) { | ||
| logger.severe(String.format("VariablesRequest: Invalid variablesReference: %d", varArgs.variablesReference)); |
There was a problem hiding this comment.
The logger usage is weird to me. I would think we provide some static method for logging, which is common way using logger.
With this, we get rid of extrace logger reference and import log level.
| * This helper class provide static methods for a default instance of <code>java.util.logging.Logger</code>,providing the normal log | ||
| * methods of <code>debug</code>, <code>info</code>, <code>warning</code>, <code>error</code>, very simple code like | ||
| * <code>Log.info("Sample trace: %d", 10)</code> is able to replace <code>logger.info(String.format("Sample trace: %d", 10))</code>. | ||
| * Using a very short name to better code fitness. |
There was a problem hiding this comment.
Need rephrase the comment here. Just keep it simple and clear.
| import java.util.logging.Logger; | ||
|
|
||
| /** | ||
| * This helper class provide static methods for a default instance of <code>java.util.logging.Logger</code>,providing the normal log |
There was a problem hiding this comment.
provide ==> provides
| /** | ||
| * Log an ERROR message. | ||
| * | ||
| * @param ex The error object |
There was a problem hiding this comment.
error exception?
| * | ||
| * @param ex The error object | ||
| * @param message The string message | ||
| * @param arguments The arguments for replacing placeholder in message. |
There was a problem hiding this comment.
Should be placeholdes, corresponding to the arguments
Similar for other places of the comments
| } | ||
|
|
||
| /** | ||
| * Log an WARN message. |
| } | ||
|
|
||
| /** | ||
| * Log an DEBUG message. |
| * Add a log Handler to receive logging messages. | ||
| * @param handler a logging Handler | ||
| */ | ||
| public static void addHandler(Handler handler) { |
There was a problem hiding this comment.
Is there any API corresponding to remove the handler?
There was a problem hiding this comment.
added
Microsoft (microsoft)
deleted a comment from
Microsoft Contribution License Agreements (msftclas)
| * <code>Log.info("Sample trace: %d", 10)</code> is able to replace <code>logger.info(String.format("Sample trace: %d", 10))</code>. | ||
| * Using a very short name to better code fitness. | ||
| */ | ||
| public final class Log { |
There was a problem hiding this comment.
Why do we need to wrap around the JDK logger? I didn't see much value added. Am I missing anything?
There was a problem hiding this comment.
This PR is mainly to focus on simplifying the logging code to better code.
There was a problem hiding this comment.
Trace user data is also include in Log.java, Log will be the only class need to record logs and user datas.
| } | ||
| } catch (IOException e) { | ||
| logger.log(Level.SEVERE, String.format("Read data from io exception: %s", e.toString()), e); | ||
| Log.error("Read data from io exception: %s", e.toString()); |
There was a problem hiding this comment.
should Log.endSession at the end of the start method.
…exception there.