Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Commit

Permalink
Fix nil panic in reconciler by setting repo when closing changesets (#…
Browse files Browse the repository at this point in the history
…13795)

This fixes the crash in this stacktrace https://github.com/sourcegraph/sourcegraph/issues/13648#issuecomment-690817025

It was only a problem in `CloseChangesets`, but I added the check to the
FakeChangesetSource in every call.
  • Loading branch information
mrnugget authored Sep 14, 2020
1 parent 3723693 commit d54d450
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
2 changes: 1 addition & 1 deletion enterprise/internal/campaigns/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func (r *reconciler) closeChangeset(ctx context.Context, tx *Store, ch *campaign
return err
}

cs := &repos.Changeset{Changeset: ch}
cs := &repos.Changeset{Changeset: ch, Repo: repo}

if err := ccs.CloseChangeset(ctx, cs); err != nil {
return errors.Wrap(err, "creating changeset")
Expand Down
19 changes: 19 additions & 0 deletions enterprise/internal/campaigns/testing/changeset_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func (s *FakeChangesetSource) CreateChangeset(ctx context.Context, c *repos.Chan
return s.ChangesetExists, s.Err
}

if c.Repo == nil {
return false, NoReposErr
}

if c.HeadRef != s.WantHeadRef {
return s.ChangesetExists, fmt.Errorf("wrong HeadRef. want=%s, have=%s", s.WantHeadRef, c.HeadRef)
}
Expand All @@ -72,6 +76,9 @@ func (s *FakeChangesetSource) UpdateChangeset(ctx context.Context, c *repos.Chan
if s.Err != nil {
return s.Err
}
if c.Repo == nil {
return NoReposErr
}

if c.BaseRef != s.WantBaseRef {
return fmt.Errorf("wrong BaseRef. want=%s, have=%s", s.WantBaseRef, c.BaseRef)
Expand Down Expand Up @@ -101,6 +108,10 @@ func (s *FakeChangesetSource) LoadChangesets(ctx context.Context, cs ...*repos.C
}

for _, c := range cs {
if c.Repo == nil {
return NoReposErr
}

if err := c.SetMetadata(s.FakeMetadata); err != nil {
return err
}
Expand All @@ -109,12 +120,20 @@ func (s *FakeChangesetSource) LoadChangesets(ctx context.Context, cs ...*repos.C
s.LoadedChangesets = append(s.LoadedChangesets, cs...)
return nil
}

var NoReposErr = errors.New("no repository set on repos.Changeset")

func (s *FakeChangesetSource) CloseChangeset(ctx context.Context, c *repos.Changeset) error {
s.CloseChangesetCalled = true

if s.Err != nil {
return s.Err
}

if c.Repo == nil {
return NoReposErr
}

s.ClosedChangesets = append(s.ClosedChangesets, c)
return nil
}
Expand Down

0 comments on commit d54d450

Please sign in to comment.