From 06399630a225128bd6df1d6316afffbe35b095dc Mon Sep 17 00:00:00 2001
From: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Date: Sat, 15 Apr 2023 15:25:12 +0900
Subject: [PATCH] remove ResultContextError

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
---
 controller/build/build.go      | 46 ++++++++++------------------------
 controller/local/controller.go |  7 +-----
 controller/remote/server.go    |  8 +-----
 3 files changed, 15 insertions(+), 46 deletions(-)

diff --git a/controller/build/build.go b/controller/build/build.go
index c9a3b24f..05616a74 100644
--- a/controller/build/build.go
+++ b/controller/build/build.go
@@ -41,6 +41,11 @@ import (
 
 const defaultTargetName = "default"
 
+// RunBuild runs the specified build and returns the result.
+//
+// NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultContext,
+// this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
+// inspect the result and debug the cause of that error.
 func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.BuildOptions, inStream io.Reader, progressMode string, statusChan chan *client.SolveStatus) (*client.SolveResponse, *build.ResultContext, error) {
 	if in.NoCache && len(in.NoCacheFilter) > 0 {
 		return nil, nil, errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together")
@@ -177,11 +182,17 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
 	resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progressMode, in.MetadataFile, statusChan)
 	err = wrapBuildError(err, false)
 	if err != nil {
-		return nil, nil, err
+		// NOTE: buildTargets can return *build.ResultContext even on error.
+		return nil, res, err
 	}
 	return resp, res, nil
 }
 
+// buildTargets runs the specified build and returns the result.
+//
+// NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultContext,
+// this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
+// inspect the result and debug the cause of that error.
 func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progressMode string, metadataFile string, statusChan chan *client.SolveStatus) (*client.SolveResponse, *build.ResultContext, error) {
 	ctx2, cancel := context.WithCancel(context.TODO())
 	defer cancel()
@@ -209,10 +220,7 @@ func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGrou
 		err = err1
 	}
 	if err != nil {
-		if res != nil {
-			err = wrapResultContext(err, res)
-		}
-		return nil, nil, err
+		return nil, res, err
 	}
 
 	if len(metadataFile) > 0 && resp != nil {
@@ -382,31 +390,3 @@ func controllerUlimitOpt2DockerUlimit(u *controllerapi.UlimitOpt) *dockeropts.Ul
 	}
 	return dockeropts.NewUlimitOpt(&values)
 }
-
-// ResultContextError is an error type used for passing ResultContext from this package
-// to the caller of RunBuild. This is only used when RunBuild fails with an error.
-// When it succeeds without error, ResultContext is returned via non-error returned value.
-//
-// Caller can extract ResultContext from the error returned by RunBuild as the following:
-//
-//	resp, res, buildErr := cbuild.RunBuild(ctx, req.Options, inR, statusChan)
-//	var re *cbuild.ResultContextError
-//	if errors.As(buildErr, &re) && re.ResultContext != nil {
-//		res = re.ResultContext
-//	}
-type ResultContextError struct {
-	ResultContext *build.ResultContext
-	error
-}
-
-// Unwrap returns the original error.
-func (e *ResultContextError) Unwrap() error {
-	return e.error
-}
-
-func wrapResultContext(wErr error, res *build.ResultContext) error {
-	if wErr == nil {
-		return nil
-	}
-	return &ResultContextError{ResultContext: res, error: wErr}
-}
diff --git a/controller/local/controller.go b/controller/local/controller.go
index c8f0ff4b..163a3a09 100644
--- a/controller/local/controller.go
+++ b/controller/local/controller.go
@@ -49,12 +49,7 @@ func (b *localController) Build(ctx context.Context, options controllerapi.Build
 	defer b.buildOnGoing.Store(false)
 
 	resp, res, buildErr := cbuild.RunBuild(ctx, b.dockerCli, options, in, progressMode, nil)
-	if buildErr != nil {
-		var re *cbuild.ResultContextError
-		if errors.As(buildErr, &re) && re.ResultContext != nil {
-			res = re.ResultContext
-		}
-	}
+	// NOTE: RunBuild can return *build.ResultContext even on error.
 	if res != nil {
 		b.buildConfig = buildConfig{
 			resultCtx:    res,
diff --git a/controller/remote/server.go b/controller/remote/server.go
index 72d0a10b..d78146b1 100644
--- a/controller/remote/server.go
+++ b/controller/remote/server.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/docker/buildx/build"
-	cbuild "github.com/docker/buildx/controller/build"
 	controllererrors "github.com/docker/buildx/controller/errdefs"
 	"github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/controller/processes"
@@ -203,12 +202,7 @@ func (m *Server) Build(ctx context.Context, req *pb.BuildRequest) (*pb.BuildResp
 	resp, res, buildErr := m.buildFunc(ctx, req.Options, inR, statusChan)
 	m.sessionMu.Lock()
 	if s, ok := m.session[ref]; ok {
-		if buildErr != nil {
-			var re *cbuild.ResultContextError
-			if errors.As(buildErr, &re) && re.ResultContext != nil {
-				res = re.ResultContext
-			}
-		}
+		// NOTE: buildFunc can return *build.ResultContext even on error (e.g. when it's implemented using (github.com/docker/buildx/controller/build).RunBuild).
 		if res != nil {
 			s.result = res
 			s.cancelBuild = cancel