Boxed errors that are nil can trigger a panic in format.Object

[matthchr]

Repro code here:

// CustomError exists just to showcase the behavior. Ignore that it's a pointless error type.
type CustomError struct {
    Details string
}

func (c *CustomError) Error() string {
    return c.Details
}

var _ error = &CustomError{}

func Function1(fails bool) error {
    if fails {
        return &CustomError{
            Details: "an error",
        }
    }

    return nil
}

func Function2(fails bool) *CustomError {
    if fails {
        return &CustomError{
            Details: "an error",
        }
    }

    return nil
}

func Test_Repro1(t *testing.T) {
    g := NewGomegaWithT(t)

    g.Expect(Function1(true)).ToNot(HaveOccurred())
}

func Test_Repro2(t *testing.T) {
    g := NewGomegaWithT(t)

    g.Expect(Function1(false)).To(HaveOccurred())
}

func Test_Repro3(t *testing.T) {
    g := NewGomegaWithT(t)

    g.Expect(Function2(true)).ToNot(HaveOccurred())
}

func Test_Repro4(t *testing.T) {
    g := NewGomegaWithT(t)

    g.Expect(Function2(false)).To(HaveOccurred())
}

Test_Repro4 panics with the following traceback:

=== RUN   Test_Repro4
--- FAIL: Test_Repro4 (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5871a0]

goroutine 19 [running]:
testing.tRunner.func1.2({0x5a9dc0, 0x76ef00})
    /usr/local/go1.20/src/testing/testing.go:1526 +0x24e
testing.tRunner.func1()
    /usr/local/go1.20/src/testing/testing.go:1529 +0x39f
panic({0x5a9dc0, 0x76ef00})
    /usr/local/go1.20/src/runtime/panic.go:884 +0x213
gomegarepro.(*CustomError).Error(0x5dc627?)
    /home/matthchr/work/temp/gomegarepro/lib_test.go:15
github.com/onsi/gomega/format.Object({0x5a5b60, 0x0}, 0x4d4ce5?)
    /home/matthchr/go/pkg/mod/github.com/onsi/gomega@v1.27.8/format/format.go:263 +0x159
github.com/onsi/gomega/matchers.(*HaveOccurredMatcher).FailureMessage(0xc00003e648?, {0x5a5b60?, 0x0?})
    /home/matthchr/go/pkg/mod/github.com/onsi/gomega@v1.27.8/matchers/have_occurred_matcher.go:30 +0x2e
github.com/onsi/gomega/internal.(*Assertion).match(0xc0000f4c00, {0x63d640, 0x7c0148}, 0x1, {0x0, 0x0, 0x0})
    /home/matthchr/go/pkg/mod/github.com/onsi/gomega@v1.27.8/internal/assertion.go:101 +0x128
github.com/onsi/gomega/internal.(*Assertion).To(0xc0000f4c00, {0x63d640, 0x7c0148}, {0x0, 0x0, 0x0})
    /home/matthchr/go/pkg/mod/github.com/onsi/gomega@v1.27.8/internal/assertion.go:62 +0xb5
gomegarepro.Test_Repro4(0x0?)
    /home/matthchr/work/temp/gomegarepro/lib_test.go:61 +0x65
testing.tRunner(0xc000083380, 0x60e458)
    /usr/local/go1.20/src/testing/testing.go:1576 +0x10b
created by testing.(*T).Run
    /usr/local/go1.20/src/testing/testing.go:1629 +0x3ea

Process finished with the exit code 1

What actually happens for Test_Repro4 is that for this code, actual is an interface{} | *CustomError (a boxed nil).

When this is passed through to func Object(object interface{}, indentation uint) string, it passes the object.(error) cast and so err.Error() is called, which panics.

This doesn't happen for Test_Repro2 (which calls an identical function to Test_Repro4 except the return type is error rather than a concrete type that implements error) because error is already an interface and so is not boxed.

A possible solution for this problem would be to ensure that the error check guards against the possibility that the error is nil, like so:

    if err, ok := object.(error); ok && !isNil(err) {
        commonRepresentation += "\n" + IndentString(err.Error(), indentation) + "\n" + indent
    }

[matthchr]

If there's agreement on the suggested solution above I'm happy to send a PR.

[onsi]

hey @matthchr - thanks for the detailed work here. your solution sounds good to me, yes please do send in a PR!

[matthchr]

PR is here: https://github.com/onsi/gomega/pull/681