Merge branch 'id-lsif-close-all-resources' into 'master'

Log errors if lsif parser failed to close

See merge request gitlab-org/gitlab-workhorse!521

internal/lsif_transformer/parser/cache.go

@@ -41,11 +41,10 @@ func (c *cache) Entry(id Id, data interface{}) error {
 }
 
 func (c *cache) Close() error {
-	if err := c.file.Close(); err != nil {
-		return err
-	}
-
-	return os.Remove(c.file.Name())
+	return combineErrors(
+		c.file.Close(),
+		os.Remove(c.file.Name()),
+	)
 }
 
 func (c *cache) setOffset(id Id) error {

internal/lsif_transformer/parser/errors.go

+package parser
+
+import (
+	"errors"
+	"strings"
+)
+
+func combineErrors(errsOrNil ...error) error {
+	var errs []error
+	for _, err := range errsOrNil {
+		if err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if len(errs) == 0 {
+		return nil
+	}
+
+	if len(errs) == 1 {
+		return errs[0]
+	}
+
+	var msgs []string
+	for _, err := range errs {
+		msgs = append(msgs, err.Error())
+	}
+
+	return errors.New(strings.Join(msgs, "\n"))
+}

internal/lsif_transformer/parser/errors_test.go

+package parser
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type customErr struct {
+	err string
+}
+
+func (e customErr) Error() string {
+	return e.err
+}
+
+func TestCombineErrors(t *testing.T) {
+	err := combineErrors(nil, errors.New("first"), nil, customErr{"second"})
+	require.EqualError(t, err, "first\nsecond")
+
+	err = customErr{"custom error"}
+	require.Equal(t, err, combineErrors(nil, err, nil))
+
+	require.Nil(t, combineErrors(nil, nil, nil))
+}

internal/lsif_transformer/parser/hovers.go

@@ -89,15 +89,11 @@ func (h *Hovers) For(refId Id) json.RawMessage {
 }
 
 func (h *Hovers) Close() error {
-	if err := h.File.Close(); err != nil {
-		return err
-	}
-
-	if err := os.Remove(h.File.Name()); err != nil {
-		return err
-	}
-
-	return h.Offsets.Close()
+	return combineErrors(
+		h.File.Close(),
+		os.Remove(h.File.Name()),
+		h.Offsets.Close(),
+	)
 }
 
 func (h *Hovers) addData(line []byte) error {

internal/lsif_transformer/parser/ranges.go

@@ -120,11 +120,10 @@ func (r *Ranges) Serialize(f io.Writer, rangeIds []Id, docs map[Id]string) error
 }
 
 func (r *Ranges) Close() error {
-	if err := r.Cache.Close(); err != nil {
-		return err
-	}
-
-	return r.Hovers.Close()
+	return combineErrors(
+		r.Cache.Close(),
+		r.Hovers.Close(),
+	)
 }
 
 func (r *Ranges) definitionPathFor(docs map[Id]string, refId Id) string {

internal/upload/rewrite.go

@@ -140,7 +140,7 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa
 		inputReader = p
 	}
 
-	inputReader, err := rew.handleLsifUpload(inputReader, opts.LocalTempPath)
+	inputReader, err := rew.handleLsifUpload(ctx, inputReader, opts.LocalTempPath, filename)
 	if err != nil {
 		return err
 	}
@@ -170,7 +170,7 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa
 	return rew.filter.ProcessFile(ctx, name, fh, rew.writer)
 }
 
-func (rew *rewriter) handleLsifUpload(reader io.Reader, tempPath string) (io.Reader, error) {
+func (rew *rewriter) handleLsifUpload(ctx context.Context, reader io.Reader, tempPath, filename string) (io.Reader, error) {
 	if rew.preauth.ProcessLsif {
 		p, err := parser.NewParser(reader, tempPath)
 		if err != nil {
@@ -183,7 +183,9 @@ func (rew *rewriter) handleLsifUpload(reader io.Reader, tempPath string) (io.Rea
 		}
 
 		if err := p.Close(); err != nil {
-			return nil, err
+			log.WithContextFields(ctx, log.Fields{
+				"filename": filename,
+			}).Print("failed to close lsif parser: " + err.Error())
 		}
 
 		return z, nil