[public-api] Convert server errors to appropriate RPC errors

Author: easyCZ

components/public-api-server/pkg/apiv1/workspace.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"github.com/gitpod-io/gitpod/public-api-server/pkg/proxy"
 	v1 "github.com/gitpod-io/gitpod/public-api/v1"
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
@@ -27,19 +28,30 @@ type WorkspaceService struct {
 }
 
 func (w *WorkspaceService) GetWorkspace(ctx context.Context, r *v1.GetWorkspaceRequest) (*v1.GetWorkspaceResponse, error) {
+	logger := ctxlogrus.Extract(ctx)
 	token, err := bearerTokenFromContext(ctx)
 	if err != nil {
 		return nil, err
 	}
 
 	server, err := w.connectionPool.Get(ctx, token)
 	if err != nil {
+		logger.WithError(err).Error("Failed to get connection to server.")
 		return nil, status.Error(codes.Internal, "failed to establish connection to downstream services")
 	}
 
 	workspace, err := server.GetWorkspace(ctx, r.GetWorkspaceId())
 	if err != nil {
-		return nil, status.Error(codes.NotFound, "failed to get workspace")
+		logger.WithError(err).Error("Failed to get workspace.")
+		converted := proxy.ConvertError(err)
+		switch status.Code(converted) {
+		case codes.PermissionDenied:
+			return nil, status.Error(codes.PermissionDenied, "insufficient permission to access workspace")
+		case codes.NotFound:
+			return nil, status.Error(codes.NotFound, "workspace does not exist")
+		default:
+			return nil, status.Error(codes.Internal, "unable to retrieve workspace")
+		}
 	}
 
 	return &v1.GetWorkspaceResponse{

components/public-api-server/pkg/apiv1/workspace_test.go

@@ -6,7 +6,7 @@ package apiv1
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"github.com/gitpod-io/gitpod/common-go/baseserver"
 	gitpod "github.com/gitpod-io/gitpod/gitpod-protocol"
 	v1 "github.com/gitpod-io/gitpod/public-api/v1"
@@ -37,24 +37,10 @@ func TestWorkspaceService_GetWorkspace(t *testing.T) {
 			foundWorkspaceID: {
 				LatestInstance: &gitpod.WorkspaceInstance{},
 				Workspace: &gitpod.Workspace{
-					BaseImageNameResolved: "",
-					BasedOnPrebuildID:     "",
-					BasedOnSnapshotID:     "",
-					Config:                nil,
-					ContentDeletedTime:    "",
-					Context:               nil,
-					ContextURL:            contextURL,
-					CreationTime:          "",
-					Deleted:               false,
-					Description:           description,
-					ID:                    foundWorkspaceID,
-					ImageNameResolved:     "",
-					ImageSource:           nil,
-					OwnerID:               ownerID,
-					Pinned:                false,
-					Shareable:             false,
-					SoftDeleted:           "",
-					Type:                  "",
+					ContextURL:  contextURL,
+					Description: description,
+					ID:          foundWorkspaceID,
+					OwnerID:     ownerID,
 				},
 			},
 		}},
@@ -68,39 +54,44 @@ func TestWorkspaceService_GetWorkspace(t *testing.T) {
 	client := v1.NewWorkspacesServiceClient(conn)
 	ctx := metadata.AppendToOutgoingContext(context.Background(), "authorization", bearerToken)
 
-	scenarios := []struct {
-		name string
+	type Expectation struct {
+		Code     codes.Code
+		Response *v1.GetWorkspaceResponse
+	}
 
+	scenarios := []struct {
+		name        string
 		WorkspaceID string
-
-		ErrorCode codes.Code
-		Response  *v1.GetWorkspaceResponse
+		Expect      Expectation
 	}{
 		{
 			name:        "returns a workspace when workspace is found by ID",
 			WorkspaceID: foundWorkspaceID,
-			ErrorCode:   codes.OK,
-			Response: &v1.GetWorkspaceResponse{
-				Result: &v1.Workspace{
-					WorkspaceId: foundWorkspaceID,
-					OwnerId:     ownerID,
-					ProjectId:   "",
-					Context: &v1.WorkspaceContext{
-						ContextUrl: contextURL,
-						Details: &v1.WorkspaceContext_Git_{Git: &v1.WorkspaceContext_Git{
-							NormalizedContextUrl: contextURL,
-							Commit:               "",
-						}},
+			Expect: Expectation{
+				Code: codes.OK,
+				Response: &v1.GetWorkspaceResponse{
+					Result: &v1.Workspace{
+						WorkspaceId: foundWorkspaceID,
+						OwnerId:     ownerID,
+						ProjectId:   "",
+						Context: &v1.WorkspaceContext{
+							ContextUrl: contextURL,
+							Details: &v1.WorkspaceContext_Git_{Git: &v1.WorkspaceContext_Git{
+								NormalizedContextUrl: contextURL,
+								Commit:               "",
+							}},
+						},
+						Description: description,
 					},
-					Description: description,
 				},
 			},
 		},
 		{
 			name:        "not found when workspace is not found by ID",
 			WorkspaceID: "some-not-found-workspace-id",
-			ErrorCode:   codes.NotFound,
-			Response:    nil,
+			Expect: Expectation{
+				Code: codes.NotFound,
+			},
 		},
 	}
 
@@ -109,8 +100,10 @@ func TestWorkspaceService_GetWorkspace(t *testing.T) {
 			resp, err := client.GetWorkspace(ctx, &v1.GetWorkspaceRequest{
 				WorkspaceId: scenario.WorkspaceID,
 			})
-			require.Equal(t, scenario.ErrorCode, status.Code(err), "status code must match")
-			if diff := cmp.Diff(scenario.Response, resp, protocmp.Transform()); diff != "" {
+			if diff := cmp.Diff(scenario.Expect, Expectation{
+				Code:     status.Code(err),
+				Response: resp,
+			}, protocmp.Transform()); diff != "" {
 				t.Errorf("unexpected difference:\n%v", diff)
 			}
 		})
@@ -153,7 +146,7 @@ type FakeGitpodAPI struct {
 func (f *FakeGitpodAPI) GetWorkspace(ctx context.Context, id string) (res *gitpod.WorkspaceInfo, err error) {
 	w, ok := f.workspaces[id]
 	if !ok {
-		return nil, fmt.Errorf("workspace not found")
+		return nil, errors.New("code 404")
 	}
 
 	return w, nil

components/public-api-server/pkg/proxy/errors.go

@@ -0,0 +1,35 @@
+// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License-AGPL.txt in the project root for license information.
+
+package proxy
+
+import (
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"strings"
+)
+
+func ConvertError(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	s := err.Error()
+
+	if strings.Contains(s, "code 401") {
+		return status.Error(codes.PermissionDenied, s)
+	}
+
+	// components/gitpod-protocol/src/messaging/error.ts
+	if strings.Contains(s, "code 404") {
+		return status.Error(codes.NotFound, s)
+	}
+
+	// components/gitpod-messagebus/src/jsonrpc-server.ts#47
+	if strings.Contains(s, "code -32603") {
+		return status.Error(codes.Internal, s)
+	}
+
+	return status.Error(codes.Internal, s)
+}

components/public-api-server/pkg/proxy/errors_test.go

@@ -0,0 +1,36 @@
+// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License-AGPL.txt in the project root for license information.
+
+package proxy
+
+import (
+	"errors"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"testing"
+)
+
+func TestConvertError(t *testing.T) {
+	scenarios := []struct {
+		WebsocketError error
+		ExpectedStatus codes.Code
+	}{
+		{
+			WebsocketError: errors.New("reconnecting-ws: bad handshake: code 401 - URL: wss://main.preview.gitpod-dev.com/api/v1 - headers: map[Authorization:[Bearer foo] Origin:[http://main.preview.gitpod-dev.com/]]"),
+			ExpectedStatus: codes.PermissionDenied,
+		},
+		{
+			WebsocketError: errors.New("jsonrpc2: code -32603 message: Request getWorkspace failed with message: No workspace with id 'some-id' found."),
+			ExpectedStatus: codes.Internal,
+		},
+	}
+
+	for _, s := range scenarios {
+		converted := ConvertError(s.WebsocketError)
+		require.Equal(t, s.ExpectedStatus, status.Code(converted))
+		// the error message should remain the same
+		require.Equal(t, s.WebsocketError.Error(), status.Convert(converted).Message())
+	}
+}