ddl: improve an error handling

[tiancaiamao]

var err *MyErr
e1 := errors.Trace(err)
e2 := errors.Cause(e1)
e2 != nil    // Isn't it beyond the normal comprehension?

We should never errors.Trace(xxx) when xxx is other than error type.

====================== TL;DR ===============

The minimal runnable code to demonstrate the problem:

package main

import "github.com/juju/errors"

func main() {
	var err *MyErr
	e1 := errors.Trace(err)
	e2 := errors.Cause(e1)
	if e2 != nil {
		panic(e2)
	}
}

type MyErr struct {
}

func (e *MyErr) Error() string {
	return ""
}

The problem is the interface conversion, if we pass a *MyErr type with value nil to this function

func Trace(e error) error { return e }

It's returns a error interface, which is not nil with type *MyErr, the underlying value is nil however.
This kind of data is toxic. It's easy to trap here:

if xxx, ok := e.(*MyErr); ok {
    xxx.doSomething()   // BOOM !!!
}

So the simple rule: We should never errors.Trace(xxx) when xxx is other than error type.
B.T.W, this kind of code is also dangerous:

func f() error {
    var e *MyError
    // ..... e is nil
    return e
}

@coocood @shenli

[tiancaiamao]

@zimulala

[zimulala]

I think this doesn't a good way to handle this problem.
If historyJob isn't synced, the historyJob's Error mustn't be nil. So I think something wrong with the operation of running DDL job.

[tiancaiamao]

Just change one line, where I think the error handling could be improved.
I think We should never errors.Trace(xxx) when xxx is other than error type..

Maybe I use the wrong presentation, this have nothing to do with fix, I just want to make the code more robust. So is that acceptable? @zimulala

[zimulala]

No.
I think historyJob's Error is nil is also a problem.

[zimulala]

Relate to #5751

[tiancaiamao]

/run-all-tests

[zimulala]

LGTM

[coocood]

LGTM

[coocood]

/run-all-tests