Error & Log

[taowen]

GOAL

Error and log handling should not bury the real code logic. The repetitive code should be kept minimal.

Java

try {
    FileInputStream input = new FileInputStream("input.txt");
    FileOutputStream output = new FileOutputStream("output.txt");
    copy(input, output);
    output.close();
    input.close();
} catch (Exception e) {
    writeLog(e);
    throw new RuntimeException(e);
}

there are couple things wrong here, from resource management point of view

• input should be closed no matter copy failed or not
• output should be closed no matter copy failed or not
• if copy failed, the new output file should be removed from disk

from debugging point of view

• the exception thrown contains low level error message, it might be confusing for the api caller or end-user.
• there is no context added the exception, for example the output file path might be important
• the log does not have correlation id with other logs or the end-user error message
• the log does not have context info, for example the output file path might be important

to make the code production ready, we might bloat up the code like this

try {
    FileInputStream input = new FileInputStream("input.txt");
    try {
        FileOutputStream output = new FileOutputStream("output.txt");
        try {
            copy(input, output);
        } catch (Exception e) {
            if (!new File("output.txt").delete()) {
                writeLog("failed to remove copy target", 
                        "target", "output.txt", 
                        "traceId", traceId);
            }
            throw new RuntimeException("failed to copy input.txt to output.txt", e);
        } finally {
            output.close();
        }
    } finally {
        input.close();
    }
} catch (Exception e) {
    writeLog("failed to remove copy target", 
            "exception", e, 
            "target", "output.txt",
            "traceId", traceId);
    throw new RuntimeException("failed to copy input.txt to output.txt", e);
}

This is actually a best case scenario. If you have extra monitoring/benchmarking code needed, and can not do the monitoring/benchmarking in the writeLog sub function, there will be more "non-functional" code around the actual logic copy(input, output)

The code has following issues

• open input and close input is far apart
• open output and close output is far apart
• create output file and remove output file is far apart

It is very easy to forget close or remove when job is done or job failed. Java 7 introduced try-with-resource to remove finally

try(FileInputStream input = new FileInputStream("input.txt")) {
        try(FileOutputStream output = new FileOutputStream("output.txt")) {
            copy(input, output);
        } catch (Exception e) {
            if (!new File("output.txt").delete()) {
                writeLog("failed to remove copy target", 
                        "target", "output.txt", 
                        "traceId", traceId);
            }
            throw new RuntimeException("failed to copy input.txt to output.txt", e);
        }
} catch (Exception e) {
    writeLog("failed to remove copy target", 
            "exception", e, 
            "target", "output.txt",
            "traceId", traceId);
    throw new RuntimeException("failed to copy input.txt to output.txt", e);
}

Go

the same function implemented in go

func CopyFile(src, dst string) error {
    r, err := os.Open(src)
    if err != nil {
        return fmt.Errorf("copy %s %s: %v", src, dst, err)
    }
    defer r.Close()

    w, err := os.Create(dst)
    if err != nil {
        return fmt.Errorf("copy %s %s: %v", src, dst, err)
    }

    if _, err := io.Copy(w, r); err != nil {
        w.Close()
        os.Remove(dst)
        return fmt.Errorf("copy %s %s: %v", src, dst, err)
    }

    if err := w.Close(); err != nil {
        os.Remove(dst)
        return fmt.Errorf("copy %s %s: %v", src, dst, err)
    }
}

Go error handling also has similar drawbacks

• os.Create(dst) and os.Remove(dst) is far apart
• it is easy to forget error handling, default behavior is silence
• fmt.Errorf is tedious
• logging is tedious

Shiti

func CopyFile(src, dst string) {
    handle err {
        throw errors.Wrap(err=err, msg="copy {src} to {dst} failed")
    }

    let r = os.Open(file=src)
    defer r.Close()

    let w = os.Create(file=dst)
    handle err {
        w.Close()
        os.Remove(file=dst) // (only if a check fails)
    }

    io.Copy(writer=w, reader=r)
    w.Close()
}

• handle err likes the catch clause, but its will throw the error up again, instead of really catch it.
• errors can be wrapped as a new error with new message
• each error will be intercepted once in the call stack, and send to the logger with context and correlation id.

If the error need to be "caught" and keep the code execution going, we need to use check

func CopyFile(src, dst string) {
    let r, err = check os.Open(file=src)
    if err != nil {
                  // handle it 
        }
    defer r.Close()
        // ...
}