Skip to content

Handle nil cli in phabricator.NewClient

Administrator requested to merge phabricator-nil-client into master

Created by: lguychard

This fixes a go panic when processing the resolvePhabricatorDiff graphql mutation.

Stack trace 
graphql: panic occurred: runtime error: invalid memory address or nil pointer dereference
goroutine 11260 [running]:
github.com/graph-gophers/graphql-go/log.(*DefaultLogger).LogPanic(0x3a5ca30, 0x29b18a0, 0xc003d330e0, 0x22784c0, 0x39fd440)
      /Users/loicguychard/go/pkg/mod/github.com/graph-gophers/[email protected]/log/log.go:21 +0x6d
github.com/graph-gophers/graphql-go/internal/exec.execFieldSelection.func2.1(0xc000248540, 0x29b18a0, 0xc003d330e0, 0xc00114d1a0, 0xc000472f20)
      /Users/loicguychard/go/pkg/mod/github.com/graph-gophers/[email protected]/internal/exec/exec.go:179 +0x8d
panic(0x22784c0, 0x39fd440)
      /usr/local/Cellar/go/1.13/libexec/src/runtime/panic.go:679 +0x1b2
github.com/sourcegraph/sourcegraph/pkg/httpcli.HeadersMiddleware.func1.1(0xc000eb0600, 0x245b400, 0x2557301, 0xc0037f8f80)
      /Users/loicguychard/repos/sourcegraph/pkg/httpcli/client.go:117 +0x1b5
github.com/sourcegraph/sourcegraph/pkg/httpcli.DoerFunc.Do(0xc003220120, 0xc000eb0600, 0x0, 0x0, 0xc001dd8ba0)
      /Users/loicguychard/repos/sourcegraph/pkg/httpcli/client.go:28 +0x30
github.com/uber/gonduit/core.PerformCallContext(0x29b18a0, 0xc003d33230, 0xc00297e450, 0x2c, 0x0, 0x0, 0x2173e00, 0xc001dd8c00, 0xc001dd8ba0, 0x0, ...)
      /Users/loicguychard/go/pkg/mod/github.com/sourcegraph/[email protected]/core/call.go:38 +0x1d6
github.com/uber/gonduit.(*Dialer).DialContext(0xc003220180, 0x29b18a0, 0xc003d33230, 0xc000423080, 0x10, 0xc001dd8ba0, 0x22adba0, 0xc003220100, 0x5658510)
      /Users/loicguychard/go/pkg/mod/github.com/sourcegraph/[email protected]/dialer.go:54 +0xdc
github.com/uber/gonduit.DialContext(0x29b18a0, 0xc003d33230, 0xc000423080, 0x10, 0xc001dd8ba0, 0xa, 0x0, 0x0)
      /Users/loicguychard/go/pkg/mod/github.com/sourcegraph/[email protected]/dialer.go:32 +0xac
github.com/sourcegraph/sourcegraph/pkg/extsvc/phabricator.NewClient(0x29b18a0, 0xc003d33230, 0xc000423080, 0x10, 0xc003878e00, 0x20, 0x0, 0x0, 0x31, 0xe0, ...)
      /Users/loicguychard/repos/sourcegraph/pkg/extsvc/phabricator/client.go:30 +0x1b8
github.com/sourcegraph/sourcegraph/cmd/frontend/graphqlbackend.makePhabClientForOrigin(0x29b18a0, 0xc003d33230, 0xc0015032e0, 0x10, 0x1, 0x31, 0x0)
      /Users/loicguychard/repos/sourcegraph/cmd/frontend/graphqlbackend/repository.go:408 +0x1f0
github.com/sourcegraph/sourcegraph/cmd/frontend/graphqlbackend.(*schemaResolver).ResolvePhabricatorDiff(0xc0018180a0, 0x29b18a0, 0xc003d33230, 0xc0030f8500, 0x0, 0x0, 0x0)

I poked around and noticed that makePhabClientForOrigin passed a nil cli to phabricator.NewClient. I applied a fix similar to what is done in github.NewClient, which solved the issue.

Merge request reports