RoboTutorLLC / RoboTutor_2020

Other
3 stars 1 forks source link

Feature request: RoboTutor, RTFaceLogin, RoboFeel, etc., log stack traces for every ERROR #125

Closed JackMostow closed 1 year ago

JackMostow commented 1 year ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. ERROR logged with insufficient information to debug.

Note: ERROR = uncaught exception logged as ERROR CRASH = uncaught exception.

app/src/main/java/cmu/xprize/robotutor/tutorengine/util/CrashHandler.java: public class CrashHandler implements Thread.UncaughtExceptionHandler {

comp_logging/src/main/java/cmu/xprize/comp_logging/CErrorManager.java

In https://github.com/RoboTutorLLC/RoboTutor_2020/blob/bfa7cc68b00edda06dac0eb8756b1fb81b5318ad/comp_logging/src/main/java/cmu/xprize/comp_logging/CLogManagerBase.java:

public void postError(String Tag, String Msg, Exception e) {
    String packet;

    packet = "{" +
            "\"class\":\"ERROR\"," +
            "\"tag\":\"" + Tag + "\"," +
            "\"type\":\"Exception\"," +
            "\"time\":\"" + System.currentTimeMillis() + "\"," +
            "\"msg\":\"" + Msg + "\"," +
            "\"exception\":\"" + e.toString() + "\"" +
            "},\n";

    post(packet);
}

Describe the solution you'd like A clear and concise description of what you want to happen. Log ERROR_message .txt like CRASH logs containing stack trace. Do either in postError or in whoever calls postError. Include sanitized error message in filename to make it easier to find instances of the same error message. To avoid infinite loop, don't do it while already in the middle of logging another stack trace.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered. a. 1-stop shopping: modify a single global ERROR function if it exists. b. Cherrypick specific errors to stack-trace, e.g. in animator graph. c. transform all source code to call an instrumented ERROR instead current ERROR. d. In postError or whoever calls it: dump stack trace if not already doing it. --> DO THIS

Approach to be followed (optional) A clear and concise description of approach to be followed.

Additional context Add any other context or screenshots about the feature request here.

JackMostow commented 1 year ago

From bhargav 10:01 AM (6 hours ago) to me, Rangoju

Dear Professor,

I read through this issue #125 . Do we need to improve the format in which we log the ERROR, so that debugging is easy?

Thank you

M BHARGAV

JackMostow commented 1 year ago

Good question! Yes. Stack traces in ERROR logs should look the same as in CRASH logs. In fact CRASH is a special case of ERROR, for otherwise unhandled exceptions. It should be easy to generalize the scripts that digest CRASH logs to work for ERROR logs as well.

Please see 2023-01-12 Meeting with Madelein and Rohan.

In addition to error message, tablet ID, and timestamp, the ERROR filename should include the session ID (tablet ID + sequence number + launch timestamp + anything else?) to make it easy to find the log for the session where the error occurred. This matching was problematic for CRASH, aggravated by missing logs.

The ERROR log itself should not contain timestamps or any other information specific to a particular instance of the error. Then we can generate a table with one ERROR per row (by replacing newline with a different character), and add a pivot table to count the number of instances of each ERROR.

JackMostow commented 1 year ago

All-error stack traces implemented for RoboTutor_2020; next do for RTFaceLogin, RoboFeel

madeleinvillegas commented 1 year ago

Closing because Bhargav already implemented