ENG-1461: Show a clear error when the build log streamer connection is refused

pkg/platform/api/buildlogstream/streamer.go

@@ -1,6 +1,7 @@
 package buildlogstream
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/gorilla/websocket"
@@ -18,6 +19,16 @@ import (
 // keeping the token out of proxy/browser response logs.
 const wsSubprotocol = "build-log-streamer.activestate.com.v1"
 
+// StreamDeniedError indicates the server refused the build-log stream.
+type StreamDeniedError struct {
+	*errs.WrapperError
+}
+
+func IsStreamDenied(err error) bool {
+	var e *StreamDeniedError
+	return errors.As(err, &e)
+}
+
 // Connect opens the build-log-streamer WebSocket. When jwt is non-empty it is
 // offered via Sec-WebSocket-Protocol as `bearer.<jwt>` (alongside
 // wsSubprotocol, which the server echoes back) so the server can authorize the
@@ -40,8 +51,11 @@ func Connect(ctx context.Context, jwt string) (*websocket.Conn, error) {
 	}
 
 	logging.Debug("Creating websocket for %s (origin: %s)", url.String(), header.Get("Origin"))
-	conn, _, err := dialer.DialContext(ctx, url.String(), header)
+	conn, resp, err := dialer.DialContext(ctx, url.String(), header)
 	if err != nil {
+		if resp != nil && (resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden) {
+			return nil, &StreamDeniedError{errs.Wrap(err, "build-log-streamer WebSocket Upgrade denied with status %d", resp.StatusCode)}
+		}
 		return nil, errs.Wrap(err, "Could not create websocket dialer")
 	}
 	return conn, nil

pkg/platform/api/buildlogstream/streamer_test.go

@@ -2,18 +2,28 @@ package buildlogstream
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/ActiveState/cli/internal/constants"
+	"github.com/ActiveState/cli/internal/errs"
 	"github.com/gorilla/websocket"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestIsStreamDenied(t *testing.T) {
+	denied := &StreamDeniedError{errs.New("denied")}
+	assert.True(t, IsStreamDenied(denied), "a StreamDeniedError must be recognized")
+	assert.True(t, IsStreamDenied(errs.Wrap(denied, "wrapped")), "denial must be recognized through wrapping")
+	assert.False(t, IsStreamDenied(errs.New("some other failure")), "an unrelated error must not be a denial")
+	assert.False(t, IsStreamDenied(nil), "nil must not be a denial")
+}
+
 // upgradeRequest captures the headers the build-log-streamer server saw on the
 // WS Upgrade. The mock handler writes the fields from the server goroutine and
 // closes recorded; callers must await() before reading the fields so there's a
@@ -93,6 +103,46 @@ func TestConnect_ForwardsJWTViaSubprotocol(t *testing.T) {
 		"client must send the versioned State Tool User-Agent so the server can monitor versions")
 }
 
+// startDenyingBLS stands up a server that refuses the WS Upgrade with the given
+// HTTP status, and redirects Connect's resolved service URL at it.
+func startDenyingBLS(t *testing.T, status int) {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(status)
+	}))
+	t.Cleanup(srv.Close)
+
+	wsURL := "ws" + strings.TrimPrefix(srv.URL, "http")
+	t.Setenv(constants.APIServiceOverrideEnvVarName+"BUILDLOG_STREAMER", wsURL)
+}
+
+func TestConnect_UpgradeDeniedReturnsTypedError(t *testing.T) {
+	for _, status := range []int{http.StatusUnauthorized, http.StatusForbidden} {
+		status := status
+		t.Run(http.StatusText(status), func(t *testing.T) {
+			startDenyingBLS(t, status)
+
+			_, err := Connect(context.Background(), "header.payload.signature")
+			require.Error(t, err)
+			var denied *StreamDeniedError
+			assert.Truef(t, errors.As(err, &denied),
+				"a %d Upgrade response must be classified as denial, got: %v", status, err)
+		})
+	}
+}
+
+func TestConnect_NonAuthDialErrorNotDenied(t *testing.T) {
+	// A handshake failure that isn't an auth rejection is a genuine error and
+	// must not be mistaken for a denial (the run should still surface it).
+	startDenyingBLS(t, http.StatusInternalServerError)
+
+	_, err := Connect(context.Background(), "")
+	require.Error(t, err)
+	var denied *StreamDeniedError
+	assert.False(t, errors.As(err, &denied),
+		"a non-auth handshake failure must not be classified as denial")
+}
+
 func TestConnect_AnonymousOffersNoBearer(t *testing.T) {
 	got := startMockBLS(t)
 

pkg/runtime/internal/buildlog/buildlog.go

@@ -2,6 +2,7 @@ package buildlog
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -102,6 +103,10 @@ func (b *BuildLog) OnArtifactReady(id strfmt.UUID, cb func()) {
 func (b *BuildLog) Wait(ctx context.Context) error {
 	conn, err := buildlogstream.Connect(ctx, b.authToken)
 	if err != nil {
+		var denied *buildlogstream.StreamDeniedError
+		if errors.As(err, &denied) {
+			return denied
+		}
 		return errs.Wrap(err, "Could not connect to build-log streamer build updates")
 	}
 
@@ -124,6 +129,10 @@ func (b *BuildLog) Wait(ctx context.Context) error {
 		if err == nil {
 			continue
 		}
+		var denied *buildlogstream.StreamDeniedError
+		if errors.As(err, &denied) {
+			return denied // this is a singular error that arrives alone
+		}
 		if rerr == nil {
 			rerr = errs.New("failed build")
 		}
@@ -223,16 +232,24 @@ func (b *BuildLog) waitForBuildLog(ctx context.Context, conn *websocket.Conn, er
 		}
 	}
 
+	receivedFrame := false
 	var artifactErr error
 	for {
 		var msg Message
 		err := conn.ReadJSON(&msg)
 		if err != nil {
+			// At this time, the server can either deny the websocket connect, or accept it and close it
+			// without sending any frames. We handle the latter here.
+			if !receivedFrame && websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway) {
+				errCh <- &buildlogstream.StreamDeniedError{errs.Wrap(err, "build-log stream soft-closed with no frames")}
+				return
+			}
 			// This should bubble up and logging it is just an extra measure to help with debugging
 			logging.Debug("Encountered error: %s", errs.JoinMessage(err))
 			errCh <- err
 			return
 		}
+		receivedFrame = true
 		if verboseLogging {
 			logging.Debug("Received response: %s", msg.MessageTypeValue())
 		}

pkg/runtime/internal/buildlog/buildlog_denial_test.go

@@ -0,0 +1,49 @@
+package buildlog
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/ActiveState/cli/internal/constants"
+	"github.com/ActiveState/cli/pkg/buildplan"
+	"github.com/ActiveState/cli/pkg/platform/api/buildlogstream"
+	"github.com/go-openapi/strfmt"
+	"github.com/gorilla/websocket"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestWait_SoftCloseIsDenial drives Wait against a server that accepts the
+// Upgrade then soft-closes with no frames (one of the two deny shapes), and
+// asserts the run degrades to a denial instead of surfacing a build failure.
+func TestWait_SoftCloseIsDenial(t *testing.T) {
+	upgrader := websocket.Upgrader{CheckOrigin: func(*http.Request) bool { return true }}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		conn, err := upgrader.Upgrade(w, r, nil)
+		if err != nil {
+			t.Errorf("mock build-log-streamer failed to upgrade: %v", err)
+			return
+		}
+		// Drain the client's recipe request, then close normally with no build
+		// frames -- the server-side soft-close denial shape.
+		_, _, _ = conn.ReadMessage()
+		_ = conn.WriteControl(websocket.CloseMessage,
+			websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""),
+			time.Now().Add(5*time.Second))
+		_ = conn.Close()
+	}))
+	t.Cleanup(srv.Close)
+
+	wsURL := "ws" + strings.TrimPrefix(srv.URL, "http")
+	t.Setenv(constants.APIServiceOverrideEnvVarName+"BUILDLOG_STREAMER", wsURL)
+
+	blog := New(strfmt.UUID("00000000-0000-0000-0000-000000000000"), buildplan.ArtifactIDMap{}, "")
+
+	err := blog.Wait(context.Background())
+	require.Error(t, err)
+	assert.Truef(t, buildlogstream.IsStreamDenied(err), "a soft-close with no frames must be recognized as a denial, got: %v", err)
+}

pkg/runtime/setup.go

@@ -29,6 +29,7 @@ import (
 	"github.com/ActiveState/cli/internal/svcctl"
 	"github.com/ActiveState/cli/internal/unarchiver"
 	"github.com/ActiveState/cli/pkg/buildplan"
+	"github.com/ActiveState/cli/pkg/platform/api/buildlogstream"
 	"github.com/ActiveState/cli/pkg/platform/api/buildplanner/types"
 	"github.com/ActiveState/cli/pkg/platform/model"
 	"github.com/ActiveState/cli/pkg/runtime/events"
@@ -303,6 +304,14 @@ func (s *setup) update() error {
 	// Wait for build to finish
 	if !s.buildplan.IsBuildReady() && len(s.toBuild) > 0 {
 		if err := blog.Wait(context.Background()); err != nil {
+			if buildlogstream.IsStreamDenied(err) {
+				if s.opts.AuthToken == "" {
+					return locale.WrapExternalError(err, "err_buildlog_stream_denied_unauthenticated",
+						"Could not monitor in-progress build. Please authenticate by running '[ACTIONABLE]state auth[/RESET]' and try again.")
+				}
+				return locale.WrapExternalError(err, "err_buildlog_stream_denied_unauthorized",
+					"Could not monitor in-progress build. If this is a private project, make sure your account has access to it.")
+			}
 			return errs.Wrap(err, "errors occurred during buildlog streaming")
 		}
 	}