From 6b99102fcd4c209e1572ddc5ed4a74b42aed03e2 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Wed, 9 Oct 2024 13:18:51 -0600
Subject: [PATCH 1/9] Better messaging for missing requirements.txt or
renv.lock
---
extensions/vscode/src/eventErrors.test.ts | 36 ++++++
extensions/vscode/src/eventErrors.ts | 33 ++++-
extensions/vscode/src/utils/errorTypes.ts | 4 +-
internal/clients/connect/capabilities.go | 16 ++-
internal/clients/connect/client_connect.go | 4 +-
.../clients/connect/client_connect_test.go | 108 +++++++++++++++-
.../connect/testdata/missing-reqs/app.py | 120 ++++++++++++++++++
.../connect/testdata/python-bundle/app.py | 120 ++++++++++++++++++
.../testdata/python-bundle/requirements.txt | 6 +
internal/publish/r_package_descriptions.go | 11 ++
.../publish/r_package_descriptions_test.go | 21 +++
internal/types/error.go | 2 +
12 files changed, 466 insertions(+), 15 deletions(-)
create mode 100644 internal/clients/connect/testdata/missing-reqs/app.py
create mode 100644 internal/clients/connect/testdata/python-bundle/app.py
create mode 100644 internal/clients/connect/testdata/python-bundle/requirements.txt
diff --git a/extensions/vscode/src/eventErrors.test.ts b/extensions/vscode/src/eventErrors.test.ts
index 1763560df..51215f20d 100644
--- a/extensions/vscode/src/eventErrors.test.ts
+++ b/extensions/vscode/src/eventErrors.test.ts
@@ -7,6 +7,8 @@ import {
isCodedEventErrorMessage,
isEvtErrDeploymentFailed,
isEvtErrRenvLockPackagesReadingFailed,
+ isEvtErrRequirementsReadingFailed,
+ isEvtErrDeployedContentNotRunning,
handleEventCodedError,
} from "./eventErrors";
import { ErrorCode } from "./utils/errorTypes";
@@ -66,6 +68,30 @@ describe("Event errors", () => {
expect(result).toBe(true);
});
+ test("isEvtErrRequirementsReadingFailed", () => {
+ // Message with another error code
+ let streamMsg = mkEventStreamMsg({}, "unknown");
+ let result = isEvtErrRequirementsReadingFailed(streamMsg);
+ expect(result).toBe(false);
+
+ // Message with error code
+ streamMsg = mkEventStreamMsg({}, "requirementsFileReadingError");
+ result = isEvtErrRequirementsReadingFailed(streamMsg);
+ expect(result).toBe(true);
+ });
+
+ test("isEvtErrDeployedContentNotRunning", () => {
+ // Message with another error code
+ let streamMsg = mkEventStreamMsg({}, "unknown");
+ let result = isEvtErrDeployedContentNotRunning(streamMsg);
+ expect(result).toBe(false);
+
+ // Message with error code
+ streamMsg = mkEventStreamMsg({}, "deployedContentNotRunning");
+ result = isEvtErrDeployedContentNotRunning(streamMsg);
+ expect(result).toBe(true);
+ });
+
test("handleEventCodedError", () => {
const msgData = {
dashboardUrl: "https://here.it.is/content/abcdefg",
@@ -86,5 +112,15 @@ describe("Event errors", () => {
streamMsg = mkEventStreamMsg(msgData, "renvlockPackagesReadingError");
resultMsg = handleEventCodedError(streamMsg);
expect(resultMsg).toBe("Could not scan R packages from renv lockfile");
+
+ msgData.message = "requirements.txt is missing";
+ streamMsg = mkEventStreamMsg(msgData, "requirementsFileReadingError");
+ resultMsg = handleEventCodedError(streamMsg);
+ expect(resultMsg).toBe("requirements.txt is missing");
+
+ msgData.message = "deployed content not running";
+ streamMsg = mkEventStreamMsg(msgData, "deployedContentNotRunning");
+ resultMsg = handleEventCodedError(streamMsg);
+ expect(resultMsg).toBe("deployed content not running");
});
});
diff --git a/extensions/vscode/src/eventErrors.ts b/extensions/vscode/src/eventErrors.ts
index b62c74ab1..32d40241d 100644
--- a/extensions/vscode/src/eventErrors.ts
+++ b/extensions/vscode/src/eventErrors.ts
@@ -25,6 +25,10 @@ type lockfileReadingEvtErr = baseEvtErr & {
lockfile: string;
};
+type requirementsReadingEvtErr = baseEvtErr & {
+ requirementsFile: string;
+};
+
export const isEvtErrDeploymentFailed = (
emsg: EventStreamMessageErrorCoded,
): emsg is EventStreamMessageErrorCoded<baseEvtErr> => {
@@ -37,14 +41,33 @@ export const isEvtErrRenvLockPackagesReadingFailed = (
return emsg.errCode === "renvlockPackagesReadingError";
};
+export const isEvtErrRequirementsReadingFailed = (
+ emsg: EventStreamMessageErrorCoded,
+): emsg is EventStreamMessageErrorCoded<requirementsReadingEvtErr> => {
+ return emsg.errCode === "requirementsFileReadingError";
+};
+
+export const isEvtErrDeployedContentNotRunning = (
+ emsg: EventStreamMessageErrorCoded,
+): emsg is EventStreamMessageErrorCoded<baseEvtErr> => {
+ return emsg.errCode === "deployedContentNotRunning";
+};
+
+export const useEvtErrKnownMessage = (
+ emsg: EventStreamMessageErrorCoded,
+): boolean => {
+ return (
+ isEvtErrDeploymentFailed(emsg) ||
+ isEvtErrRenvLockPackagesReadingFailed(emsg) ||
+ isEvtErrRequirementsReadingFailed(emsg) ||
+ isEvtErrDeployedContentNotRunning(emsg)
+ );
+};
+
export const handleEventCodedError = (
emsg: EventStreamMessageErrorCoded,
): string => {
- if (isEvtErrDeploymentFailed(emsg)) {
- return emsg.data.message;
- }
-
- if (isEvtErrRenvLockPackagesReadingFailed(emsg)) {
+ if (useEvtErrKnownMessage(emsg)) {
return emsg.data.message;
}
diff --git a/extensions/vscode/src/utils/errorTypes.ts b/extensions/vscode/src/utils/errorTypes.ts
index f78b67237..be8d47556 100644
--- a/extensions/vscode/src/utils/errorTypes.ts
+++ b/extensions/vscode/src/utils/errorTypes.ts
@@ -10,7 +10,9 @@ export type ErrorCode =
| "invalidConfigFile"
| "errorCertificateVerification"
| "deployFailed"
- | "renvlockPackagesReadingError";
+ | "renvlockPackagesReadingError"
+ | "requirementsFileReadingError"
+ | "deployedContentNotRunning";
export type axiosErrorWithJson<T = { code: ErrorCode; details: unknown }> =
AxiosError & {
diff --git a/internal/clients/connect/capabilities.go b/internal/clients/connect/capabilities.go
index f70d27407..c6144d290 100644
--- a/internal/clients/connect/capabilities.go
+++ b/internal/clients/connect/capabilities.go
@@ -26,11 +26,14 @@ type allSettings struct {
}
const requirementsFileMissing = `
-can't find the package file (%s) in the project directory.
-Create the file, listing the packages your project depends on.
-Or scan your project dependencies using scan button in
-the Python Packages section of the UI and review the
-generated file`
+Missing package file %s. The file must exist and be included in the deployment.
+Create the file listing your project dependencies.
+Or do an automatic scan with the help of the Python Packages section
+of the Publisher extension and review the generated file`
+
+type requirementsErrDetails struct {
+ RequirementsFile string `json:"requirements_file"`
+}
func checkRequirementsFile(base util.AbsolutePath, requirementsFilename string) error {
packageFile := base.Join(requirementsFilename)
@@ -39,7 +42,8 @@ func checkRequirementsFile(base util.AbsolutePath, requirementsFilename string)
return err
}
if !exists {
- return fmt.Errorf(requirementsFileMissing, requirementsFilename)
+ missingErr := fmt.Errorf(requirementsFileMissing, requirementsFilename)
+ return types.NewAgentError(types.ErrorRequirementsFileReading, missingErr, requirementsErrDetails{RequirementsFile: packageFile.String()})
}
return nil
}
diff --git a/internal/clients/connect/client_connect.go b/internal/clients/connect/client_connect.go
index e0e247c6f..27c6d1fa9 100644
--- a/internal/clients/connect/client_connect.go
+++ b/internal/clients/connect/client_connect.go
@@ -462,7 +462,7 @@ func (c *ConnectClient) WaitForTask(taskID types.TaskID, log logging.Logger) err
}
}
-var errValidationFailed = errors.New("couldn't access the deployed content; see the logs in Connect for details")
+var errValidationFailed = errors.New("deployed content does not seem to be running. See the logs in Connect for details")
func (c *ConnectClient) ValidateDeployment(contentID types.ContentID, log logging.Logger) error {
url := fmt.Sprintf("/content/%s/", contentID)
@@ -475,7 +475,7 @@ func (c *ConnectClient) ValidateDeployment(contentID types.ContentID, log loggin
if ok {
if httpErr.Status >= 500 {
// Validation failed - the content is not up and running
- return errValidationFailed
+ return types.NewAgentError(types.ErrorDeployedContentNotRunning, errValidationFailed, nil)
} else {
// Other HTTP codes are acceptable, for example
// if the content doesn't implement GET /,
diff --git a/internal/clients/connect/client_connect_test.go b/internal/clients/connect/client_connect_test.go
index c27b30fef..adc454697 100644
--- a/internal/clients/connect/client_connect_test.go
+++ b/internal/clients/connect/client_connect_test.go
@@ -11,11 +11,14 @@ import (
"github.com/mitchellh/mapstructure"
"github.com/posit-dev/publisher/internal/accounts"
+ "github.com/posit-dev/publisher/internal/clients/connect/server_settings"
"github.com/posit-dev/publisher/internal/clients/http_client"
+ "github.com/posit-dev/publisher/internal/config"
"github.com/posit-dev/publisher/internal/events"
"github.com/posit-dev/publisher/internal/logging"
"github.com/posit-dev/publisher/internal/logging/loggingtest"
"github.com/posit-dev/publisher/internal/types"
+ "github.com/posit-dev/publisher/internal/util"
"github.com/posit-dev/publisher/internal/util/utiltest"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
@@ -360,7 +363,7 @@ func (s *ConnectClientSuite) TestValidateDeploymentAppFailure() {
}
contentID := types.ContentID("myContentID")
err := client.ValidateDeployment(contentID, logging.New())
- s.ErrorContains(err, "couldn't access the deployed content")
+ s.ErrorContains(err, "deployed content does not seem to be running. See the logs in Connect for details")
}
func (s *ConnectClientSuite) TestValidateDeploymentHTTPNonAppErr() {
@@ -590,3 +593,106 @@ func (s *ConnectClientSuite) TestContentDetails() {
s.ErrorIs(err, expectedErr)
httpClient.AssertExpectations(s.T())
}
+
+func (s *ConnectClientSuite) TestCheckCapabilities() {
+ lgr := logging.New()
+ httpClient := &http_client.MockHTTPClient{}
+ httpClient.On("Get", "/__api__/v1/user", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/server_settings", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/server_settings/applications", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/server_settings/scheduler/python-dash", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/v1/server_settings/r", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/v1/server_settings/quarto", mock.Anything, lgr).Return(nil)
+
+ // Be sure to mock available python versions
+ httpClient.On("Get", "/__api__/v1/server_settings/python", mock.AnythingOfType("*server_settings.PyInfo"), lgr).Run(func(args mock.Arguments) {
+ pySettings := args.Get(1).(*server_settings.PyInfo)
+ pySettings.Installations = []server_settings.PyInstallation{{Version: "3.4.5"}}
+ }).Return(nil)
+
+ cfg := config.New()
+ cfg.Type = "python-dash"
+ cfg.Entrypoint = "app.py"
+ cfg.Python = &config.Python{
+ Version: "3.4.5",
+ PackageManager: "pip",
+ }
+
+ var cwd util.AbsolutePath
+ bundleTestPath := cwd.Join("testdata", "python-bundle")
+
+ client := &ConnectClient{
+ client: httpClient,
+ }
+
+ err := client.CheckCapabilities(bundleTestPath, cfg, lgr)
+ s.NoError(err)
+ httpClient.AssertExpectations(s.T())
+}
+
+func (s *ConnectClientSuite) TestCheckCapabilities_missingPythonVer() {
+ lgr := logging.New()
+ httpClient := &http_client.MockHTTPClient{}
+ httpClient.On("Get", "/__api__/v1/user", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/server_settings", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/server_settings/applications", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/server_settings/scheduler/python-dash", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/v1/server_settings/r", mock.Anything, lgr).Return(nil)
+ httpClient.On("Get", "/__api__/v1/server_settings/quarto", mock.Anything, lgr).Return(nil)
+
+ // Mock versions, not including 3.4.5
+ httpClient.On("Get", "/__api__/v1/server_settings/python", mock.AnythingOfType("*server_settings.PyInfo"), lgr).Run(func(args mock.Arguments) {
+ pySettings := args.Get(1).(*server_settings.PyInfo)
+ pySettings.Installations = []server_settings.PyInstallation{{Version: "3.3.0"}}
+ }).Return(nil)
+
+ cfg := config.New()
+ cfg.Type = "python-dash"
+ cfg.Entrypoint = "app.py"
+ cfg.Python = &config.Python{
+ Version: "3.4.5", // Not included in server settings
+ PackageManager: "pip",
+ }
+
+ var cwd util.AbsolutePath
+ bundleTestPath := cwd.Join("testdata", "python-bundle")
+
+ client := &ConnectClient{
+ client: httpClient,
+ }
+
+ err := client.CheckCapabilities(bundleTestPath, cfg, lgr)
+ s.NotNil(err)
+ s.Contains(err.Error(), "Python 3.4 is not available on the server.")
+ httpClient.AssertExpectations(s.T())
+}
+
+func (s *ConnectClientSuite) TestCheckCapabilities_missingRequirementsFile() {
+ lgr := logging.New()
+ httpClient := &http_client.MockHTTPClient{}
+
+ cfg := config.New()
+ cfg.Type = "python-dash"
+ cfg.Entrypoint = "app.py"
+ cfg.Python = &config.Python{
+ Version: "3.4.5",
+ PackageManager: "pip",
+ PackageFile: "requirements.txt",
+ }
+
+ var cwd util.AbsolutePath
+ bundleTestPath := cwd.Join("testdata", "missing-reqs")
+
+ client := &ConnectClient{
+ client: httpClient,
+ }
+
+ err := client.CheckCapabilities(bundleTestPath, cfg, lgr)
+ s.NotNil(err)
+ s.Contains(err.Error(), "Missing package file requirements.txt. The file must exist and be included in the deployment.")
+
+ aerr, yes := types.IsAgentError(err)
+ s.Equal(yes, true)
+ s.Equal(aerr.Code, types.ErrorRequirementsFileReading)
+ s.Contains(aerr.Message, "Missing package file requirements.txt. The file must exist and be included in the deployment.")
+}
diff --git a/internal/clients/connect/testdata/missing-reqs/app.py b/internal/clients/connect/testdata/missing-reqs/app.py
new file mode 100644
index 000000000..de7f34a05
--- /dev/null
+++ b/internal/clients/connect/testdata/missing-reqs/app.py
@@ -0,0 +1,120 @@
+import dash
+import dash_core_components as dcc
+import dash_html_components as html
+from dash.dependencies import Input, Output
+
+import pandas as pd
+
+external_stylesheets = ["https://codepen.io/chriddyp/pen/bWLwgP.css"]
+
+app = dash.Dash(__name__, external_stylesheets=external_stylesheets)
+
+df = pd.read_csv("https://plotly.github.io/datasets/country_indicators.csv")
+
+available_indicators = df["Indicator Name"].unique()
+
+app.layout = html.Div(
+ [
+ html.Div(
+ [
+ html.Div(
+ [
+ dcc.Dropdown(
+ id="xaxis-column",
+ options=[
+ {"label": i, "value": i} for i in available_indicators
+ ],
+ value="Fertility rate, total (births per woman)",
+ ),
+ dcc.RadioItems(
+ id="xaxis-type",
+ options=[
+ {"label": i, "value": i} for i in ["Linear", "Log"]
+ ],
+ value="Linear",
+ labelStyle={"display": "inline-block"},
+ ),
+ ],
+ style={"width": "48%", "display": "inline-block"},
+ ),
+ html.Div(
+ [
+ dcc.Dropdown(
+ id="yaxis-column",
+ options=[
+ {"label": i, "value": i} for i in available_indicators
+ ],
+ value="Life expectancy at birth, total (years)",
+ ),
+ dcc.RadioItems(
+ id="yaxis-type",
+ options=[
+ {"label": i, "value": i} for i in ["Linear", "Log"]
+ ],
+ value="Linear",
+ labelStyle={"display": "inline-block"},
+ ),
+ ],
+ style={"width": "48%", "float": "right", "display": "inline-block"},
+ ),
+ ]
+ ),
+ dcc.Graph(id="indicator-graphic"),
+ dcc.Slider(
+ id="year--slider",
+ min=df["Year"].min(),
+ max=df["Year"].max(),
+ value=df["Year"].max(),
+ marks={str(year): str(year) for year in df["Year"].unique()},
+ step=None,
+ ),
+ ]
+)
+
+
+@app.callback(
+ Output("indicator-graphic", "figure"),
+ [
+ Input("xaxis-column", "value"),
+ Input("yaxis-column", "value"),
+ Input("xaxis-type", "value"),
+ Input("yaxis-type", "value"),
+ Input("year--slider", "value"),
+ ],
+)
+def update_graph(
+ xaxis_column_name, yaxis_column_name, xaxis_type, yaxis_type, year_value
+):
+ dff = df[df["Year"] == year_value]
+
+ return {
+ "data": [
+ dict(
+ x=dff[dff["Indicator Name"] == xaxis_column_name]["Value"],
+ y=dff[dff["Indicator Name"] == yaxis_column_name]["Value"],
+ text=dff[dff["Indicator Name"] == yaxis_column_name]["Country Name"],
+ mode="markers",
+ marker={
+ "size": 15,
+ "opacity": 0.5,
+ "line": {"width": 0.5, "color": "white"},
+ },
+ )
+ ],
+ "layout": dict(
+ xaxis={
+ "title": xaxis_column_name,
+ "type": "linear" if xaxis_type == "Linear" else "log",
+ },
+ yaxis={
+ "title": yaxis_column_name,
+ "type": "linear" if yaxis_type == "Linear" else "log",
+ },
+ margin={"l": 40, "b": 40, "t": 10, "r": 0},
+ hovermode="closest",
+ ),
+ }
+
+
+if __name__ == "__main__":
+ app.run_server(debug=True)
diff --git a/internal/clients/connect/testdata/python-bundle/app.py b/internal/clients/connect/testdata/python-bundle/app.py
new file mode 100644
index 000000000..de7f34a05
--- /dev/null
+++ b/internal/clients/connect/testdata/python-bundle/app.py
@@ -0,0 +1,120 @@
+import dash
+import dash_core_components as dcc
+import dash_html_components as html
+from dash.dependencies import Input, Output
+
+import pandas as pd
+
+external_stylesheets = ["https://codepen.io/chriddyp/pen/bWLwgP.css"]
+
+app = dash.Dash(__name__, external_stylesheets=external_stylesheets)
+
+df = pd.read_csv("https://plotly.github.io/datasets/country_indicators.csv")
+
+available_indicators = df["Indicator Name"].unique()
+
+app.layout = html.Div(
+ [
+ html.Div(
+ [
+ html.Div(
+ [
+ dcc.Dropdown(
+ id="xaxis-column",
+ options=[
+ {"label": i, "value": i} for i in available_indicators
+ ],
+ value="Fertility rate, total (births per woman)",
+ ),
+ dcc.RadioItems(
+ id="xaxis-type",
+ options=[
+ {"label": i, "value": i} for i in ["Linear", "Log"]
+ ],
+ value="Linear",
+ labelStyle={"display": "inline-block"},
+ ),
+ ],
+ style={"width": "48%", "display": "inline-block"},
+ ),
+ html.Div(
+ [
+ dcc.Dropdown(
+ id="yaxis-column",
+ options=[
+ {"label": i, "value": i} for i in available_indicators
+ ],
+ value="Life expectancy at birth, total (years)",
+ ),
+ dcc.RadioItems(
+ id="yaxis-type",
+ options=[
+ {"label": i, "value": i} for i in ["Linear", "Log"]
+ ],
+ value="Linear",
+ labelStyle={"display": "inline-block"},
+ ),
+ ],
+ style={"width": "48%", "float": "right", "display": "inline-block"},
+ ),
+ ]
+ ),
+ dcc.Graph(id="indicator-graphic"),
+ dcc.Slider(
+ id="year--slider",
+ min=df["Year"].min(),
+ max=df["Year"].max(),
+ value=df["Year"].max(),
+ marks={str(year): str(year) for year in df["Year"].unique()},
+ step=None,
+ ),
+ ]
+)
+
+
+@app.callback(
+ Output("indicator-graphic", "figure"),
+ [
+ Input("xaxis-column", "value"),
+ Input("yaxis-column", "value"),
+ Input("xaxis-type", "value"),
+ Input("yaxis-type", "value"),
+ Input("year--slider", "value"),
+ ],
+)
+def update_graph(
+ xaxis_column_name, yaxis_column_name, xaxis_type, yaxis_type, year_value
+):
+ dff = df[df["Year"] == year_value]
+
+ return {
+ "data": [
+ dict(
+ x=dff[dff["Indicator Name"] == xaxis_column_name]["Value"],
+ y=dff[dff["Indicator Name"] == yaxis_column_name]["Value"],
+ text=dff[dff["Indicator Name"] == yaxis_column_name]["Country Name"],
+ mode="markers",
+ marker={
+ "size": 15,
+ "opacity": 0.5,
+ "line": {"width": 0.5, "color": "white"},
+ },
+ )
+ ],
+ "layout": dict(
+ xaxis={
+ "title": xaxis_column_name,
+ "type": "linear" if xaxis_type == "Linear" else "log",
+ },
+ yaxis={
+ "title": yaxis_column_name,
+ "type": "linear" if yaxis_type == "Linear" else "log",
+ },
+ margin={"l": 40, "b": 40, "t": 10, "r": 0},
+ hovermode="closest",
+ ),
+ }
+
+
+if __name__ == "__main__":
+ app.run_server(debug=True)
diff --git a/internal/clients/connect/testdata/python-bundle/requirements.txt b/internal/clients/connect/testdata/python-bundle/requirements.txt
new file mode 100644
index 000000000..0b385ffd1
--- /dev/null
+++ b/internal/clients/connect/testdata/python-bundle/requirements.txt
@@ -0,0 +1,6 @@
+# requirements.txt auto-generated by Posit Publisher
+# using /Users/mn/.pyenv/shims/python3
+dash
+dash_core_components
+dash_html_components
+pandas
diff --git a/internal/publish/r_package_descriptions.go b/internal/publish/r_package_descriptions.go
index afea54b8a..ee195b3cf 100644
--- a/internal/publish/r_package_descriptions.go
+++ b/internal/publish/r_package_descriptions.go
@@ -3,7 +3,9 @@
package publish
import (
+ "errors"
"fmt"
+ "os"
"github.com/posit-dev/publisher/internal/bundles"
"github.com/posit-dev/publisher/internal/events"
@@ -19,6 +21,12 @@ type lockfileErrDetails struct {
Lockfile string
}
+const lockfileMissing = `Missing R lockfile %s.
+The file must exist and be included in the deployment.
+Create the file listing your project dependencies.
+Or do an automatic scan with the help of the R Packages section
+of the Publisher extension and review the generated file`
+
func (p *defaultPublisher) getRPackages() (bundles.PackageMap, error) {
op := events.PublishGetRPackageDescriptionsOp
log := p.log.WithArgs(logging.LogKeyOp, op)
@@ -37,6 +45,9 @@ func (p *defaultPublisher) getRPackages() (bundles.PackageMap, error) {
if err != nil {
agentErr := types.NewAgentError(types.ErrorRenvLockPackagesReading, err, lockfileErrDetails{Lockfile: lockfilePath.String()})
agentErr.Message = fmt.Sprintf("Could not scan R packages from lockfile: %s", lockfileString)
+ if errors.Is(err, os.ErrNotExist) {
+ agentErr.Message = fmt.Sprintf(lockfileMissing, lockfileString)
+ }
return nil, agentErr
}
log.Info("Done collecting R package descriptions")
diff --git a/internal/publish/r_package_descriptions_test.go b/internal/publish/r_package_descriptions_test.go
index 214d79b94..5cde29c37 100644
--- a/internal/publish/r_package_descriptions_test.go
+++ b/internal/publish/r_package_descriptions_test.go
@@ -4,6 +4,7 @@ package publish
import (
"errors"
+ "os"
"testing"
"github.com/posit-dev/publisher/internal/bundles"
@@ -135,3 +136,23 @@ func (s *RPackageDescSuite) TestGetRPackages_ScanPackagesError() {
s.Contains(err.(*types.AgentError).Message, "custom.lock")
s.log.AssertExpectations(s.T())
}
+
+func (s *RPackageDescSuite) TestGetRPackages_PackageFileNotFound() {
+ expectedLockfilePath := s.dirPath.Join("custom.lock")
+
+ // With a package file
+ s.stateStore.Config = &config.Config{
+ R: &config.R{
+ PackageFile: "custom.lock",
+ },
+ }
+
+ s.packageMapper.On("GetManifestPackages", s.dirPath, expectedLockfilePath).Return(bundles.PackageMap{}, os.ErrNotExist)
+
+ publisher := s.makePublisher()
+ _, err := publisher.getRPackages()
+ s.NotNil(err)
+ s.Contains(err.(*types.AgentError).Message, "Missing R lockfile custom.lock")
+ s.Contains(err.(*types.AgentError).Message, "custom.lock")
+ s.log.AssertExpectations(s.T())
+}
diff --git a/internal/types/error.go b/internal/types/error.go
index 9166df67e..4ddd13db5 100644
--- a/internal/types/error.go
+++ b/internal/types/error.go
@@ -18,6 +18,8 @@ const (
ErrorCredentialServiceUnavailable ErrorCode = "credentialsServiceUnavailable"
ErrorCertificateVerification ErrorCode = "errorCertificateVerification"
ErrorRenvLockPackagesReading ErrorCode = "renvlockPackagesReadingError"
+ ErrorRequirementsFileReading ErrorCode = "requirementsFileReadingError"
+ ErrorDeployedContentNotRunning ErrorCode = "deployedContentNotRunning"
ErrorUnknown ErrorCode = "unknown"
)
From f85385913751f10e18e4129326f9c4aee71711a7 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Wed, 9 Oct 2024 13:59:01 -0600
Subject: [PATCH 2/9] Fix bats deploy test
---
test/bats/contract/deploy.bats | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/test/bats/contract/deploy.bats b/test/bats/contract/deploy.bats
index 01e1c848e..284d19888 100644
--- a/test/bats/contract/deploy.bats
+++ b/test/bats/contract/deploy.bats
@@ -139,11 +139,10 @@ python_content_types=(
run ${EXE} deploy ${FULL_PATH}
assert_failure
assert_output --partial "\
-can't find the package file (requirements.txt) in the project directory.
-Create the file, listing the packages your project depends on.
-Or scan your project dependencies using scan button in
-the Python Packages section of the UI and review the
-generated file."
+Missing package file requirements.txt. The file must exist and be included in the deployment.
+Create the file listing your project dependencies.
+Or do an automatic scan with the help of the Python Packages section
+of the Publisher extension and review the generated file"
else
skip
fi
From 3dccb977fd8fd0d29c24f32c1713f275910e586f Mon Sep 17 00:00:00 2001
From: Bill Sager <whs@sager.org>
Date: Thu, 10 Oct 2024 16:11:03 -0700
Subject: [PATCH 3/9] PR Recommendations - code optimization and show config
type rather than app mode description
---
extensions/vscode/src/eventErrors.ts | 3 +--
internal/clients/connect/apptypes.go | 26 +++++++++++++++++++
internal/clients/connect/client_connect.go | 8 +++---
.../clients/connect/client_connect_test.go | 2 +-
4 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/extensions/vscode/src/eventErrors.ts b/extensions/vscode/src/eventErrors.ts
index a7fb65f51..fb1abc59b 100644
--- a/extensions/vscode/src/eventErrors.ts
+++ b/extensions/vscode/src/eventErrors.ts
@@ -48,6 +48,5 @@ export const handleEventCodedError = (
return emsg.data.message;
}
- const unknownErrMsg = emsg.data.error || emsg.data.message;
- return `${unknownErrMsg}`;
+ return emsg.data.error || emsg.data.message;
};
diff --git a/internal/clients/connect/apptypes.go b/internal/clients/connect/apptypes.go
index f982b871a..7c3164eee 100644
--- a/internal/clients/connect/apptypes.go
+++ b/internal/clients/connect/apptypes.go
@@ -270,3 +270,29 @@ func AppModeFromType(t config.ContentType) AppMode {
}
return mode
}
+
+var contentTypeConnectMap = map[AppMode]config.ContentType{
+ StaticMode: config.ContentTypeHTML,
+ StaticJupyterMode: config.ContentTypeJupyterNotebook,
+ JupyterVoilaMode: config.ContentTypeJupyterVoila,
+ PythonBokehMode: config.ContentTypePythonBokeh,
+ PythonDashMode: config.ContentTypePythonDash,
+ PythonFastAPIMode: config.ContentTypePythonFastAPI,
+ PythonAPIMode: config.ContentTypePythonFlask,
+ PythonShinyMode: config.ContentTypePythonShiny,
+ PythonStreamlitMode: config.ContentTypePythonStreamlit,
+ ShinyQuartoMode: config.ContentTypeQuartoShiny,
+ StaticQuartoMode: config.ContentTypeQuarto,
+ PlumberAPIMode: config.ContentTypeRPlumber,
+ ShinyMode: config.ContentTypeRShiny,
+ ShinyRmdMode: config.ContentTypeRMarkdownShiny,
+ StaticRmdMode: config.ContentTypeRMarkdown,
+}
+
+func ContentTypeFromAppMode(a AppMode) config.ContentType {
+ contentType, ok := contentTypeConnectMap[a]
+ if !ok {
+ contentType = config.ContentTypeUnknown
+ }
+ return contentType
+}
diff --git a/internal/clients/connect/client_connect.go b/internal/clients/connect/client_connect.go
index 7bb24d8ba..c5b64de74 100644
--- a/internal/clients/connect/client_connect.go
+++ b/internal/clients/connect/client_connect.go
@@ -530,10 +530,10 @@ func (c *ConnectClient) ValidateDeploymentTarget(contentID types.ContentID, cfg
// Verify content type has not changed
log.Info("Verifying content type is the same")
- existingAppMode := content.AppMode
- configAppMode := AppModeFromType(cfg.Type)
- if configAppMode != existingAppMode {
- msg := fmt.Sprintf("Content was previously deployed as '%s' but your configuration is set to '%s'. A new deployment must be needed.", existingAppMode.Description(), configAppMode.Description())
+ existingType := ContentTypeFromAppMode(content.AppMode)
+ configType := cfg.Type
+ if existingType != configType {
+ msg := fmt.Sprintf("Content was previously deployed as '%s' but your configuration is set to '%s'.", existingType, configType)
return types.NewAgentError(events.AppModeNotModifiableCode, errors.New(msg), nil)
}
diff --git a/internal/clients/connect/client_connect_test.go b/internal/clients/connect/client_connect_test.go
index e779fbf04..0d8cfbf23 100644
--- a/internal/clients/connect/client_connect_test.go
+++ b/internal/clients/connect/client_connect_test.go
@@ -775,7 +775,7 @@ func (s *ConnectClientSuite) TestValidateDeploymentTargetAppModeNotModifiableCod
}
expectedErr := types.NewAgentError(
events.DeploymentFailedCode,
- errors.New("Content was previously deployed as 'R Shiny application' but your configuration is set to 'Dash application'. A new deployment must be needed."),
+ errors.New("Content was previously deployed as 'r-shiny' but your configuration is set to 'python-dash'."),
nil)
err := client.ValidateDeploymentTarget("e8922765-4880-43cd-abc0-d59fe59b8b4b", s.cfg, lgr)
aerr, ok := types.IsAgentError(err)
From 564c3fa67ac0f3a18ecf0d85c3e8fb4a2ffbc342 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Fri, 11 Oct 2024 12:39:39 -0600
Subject: [PATCH 4/9] PR feedback no messages. Address requirements txt file
error when it is not included in configuration files list
---
internal/clients/connect/capabilities.go | 29 ++++++++-----
.../clients/connect/client_connect_test.go | 43 +++++++++++++++++--
internal/publish/r_package_descriptions.go | 6 +--
.../publish/r_package_descriptions_test.go | 3 +-
4 files changed, 61 insertions(+), 20 deletions(-)
diff --git a/internal/clients/connect/capabilities.go b/internal/clients/connect/capabilities.go
index c6144d290..c18c9330b 100644
--- a/internal/clients/connect/capabilities.go
+++ b/internal/clients/connect/capabilities.go
@@ -25,24 +25,33 @@ type allSettings struct {
quarto server_settings.QuartoInfo
}
-const requirementsFileMissing = `
-Missing package file %s. The file must exist and be included in the deployment.
-Create the file listing your project dependencies.
-Or do an automatic scan with the help of the Python Packages section
-of the Publisher extension and review the generated file`
+const requirementsFileMissing = `Missing dependency file %s. This file must be included in the deployment.`
type requirementsErrDetails struct {
RequirementsFile string `json:"requirements_file"`
}
-func checkRequirementsFile(base util.AbsolutePath, requirementsFilename string) error {
- packageFile := base.Join(requirementsFilename)
+func checkRequirementsFile(base util.AbsolutePath, cfg *config.Config) error {
+ packageFile := base.Join(cfg.Python.PackageFile)
exists, err := packageFile.Exists()
if err != nil {
return err
}
- if !exists {
- missingErr := fmt.Errorf(requirementsFileMissing, requirementsFilename)
+
+ // Confirm the package file (requirements.txt)
+ // is included in the configuration files list.
+ requirementsIsIncluded := false
+ for _, file := range cfg.Files {
+ // File paths like /requirements.txt, /some/path/requirements.txt
+ // should count as including the package file.
+ if strings.HasSuffix(file, cfg.Python.PackageFile) {
+ requirementsIsIncluded = true
+ break
+ }
+ }
+
+ if !exists || !requirementsIsIncluded {
+ missingErr := fmt.Errorf(requirementsFileMissing, cfg.Python.PackageFile)
return types.NewAgentError(types.ErrorRequirementsFileReading, missingErr, requirementsErrDetails{RequirementsFile: packageFile.String()})
}
return nil
@@ -50,7 +59,7 @@ func checkRequirementsFile(base util.AbsolutePath, requirementsFilename string)
func (c *ConnectClient) CheckCapabilities(base util.AbsolutePath, cfg *config.Config, log logging.Logger) error {
if cfg.Python != nil {
- err := checkRequirementsFile(base, cfg.Python.PackageFile)
+ err := checkRequirementsFile(base, cfg)
if err != nil {
return err
}
diff --git a/internal/clients/connect/client_connect_test.go b/internal/clients/connect/client_connect_test.go
index adc454697..132e20b90 100644
--- a/internal/clients/connect/client_connect_test.go
+++ b/internal/clients/connect/client_connect_test.go
@@ -613,8 +613,10 @@ func (s *ConnectClientSuite) TestCheckCapabilities() {
cfg := config.New()
cfg.Type = "python-dash"
cfg.Entrypoint = "app.py"
+ cfg.Files = []string{"/app.py", "/requirements.txt"}
cfg.Python = &config.Python{
Version: "3.4.5",
+ PackageFile: "requirements.txt",
PackageManager: "pip",
}
@@ -649,8 +651,10 @@ func (s *ConnectClientSuite) TestCheckCapabilities_missingPythonVer() {
cfg := config.New()
cfg.Type = "python-dash"
cfg.Entrypoint = "app.py"
+ cfg.Files = []string{"/app.py", "/requirements.txt"}
cfg.Python = &config.Python{
Version: "3.4.5", // Not included in server settings
+ PackageFile: "requirements.txt",
PackageManager: "pip",
}
@@ -667,13 +671,14 @@ func (s *ConnectClientSuite) TestCheckCapabilities_missingPythonVer() {
httpClient.AssertExpectations(s.T())
}
-func (s *ConnectClientSuite) TestCheckCapabilities_missingRequirementsFile() {
+func (s *ConnectClientSuite) TestCheckCapabilities_requirementsFileDoesNotExist() {
lgr := logging.New()
httpClient := &http_client.MockHTTPClient{}
cfg := config.New()
cfg.Type = "python-dash"
cfg.Entrypoint = "app.py"
+ cfg.Files = []string{"/app.py", "/requirements.txt"}
cfg.Python = &config.Python{
Version: "3.4.5",
PackageManager: "pip",
@@ -689,10 +694,42 @@ func (s *ConnectClientSuite) TestCheckCapabilities_missingRequirementsFile() {
err := client.CheckCapabilities(bundleTestPath, cfg, lgr)
s.NotNil(err)
- s.Contains(err.Error(), "Missing package file requirements.txt. The file must exist and be included in the deployment.")
+ s.Contains(err.Error(), "Missing dependency file requirements.txt. This file must be included in the deployment.")
+
+ aerr, yes := types.IsAgentError(err)
+ s.Equal(yes, true)
+ s.Equal(aerr.Code, types.ErrorRequirementsFileReading)
+ s.Contains(aerr.Message, "Missing dependency file requirements.txt. This file must be included in the deployment.")
+}
+
+func (s *ConnectClientSuite) TestCheckCapabilities_requirementsFileNotInConfig() {
+ lgr := logging.New()
+ httpClient := &http_client.MockHTTPClient{}
+
+ cfg := config.New()
+ cfg.Type = "python-dash"
+ cfg.Entrypoint = "app.py"
+ cfg.Files = []string{"/app.py"} // requirements file not included in config files list
+ cfg.Python = &config.Python{
+ Version: "3.4.5",
+ PackageManager: "pip",
+ PackageFile: "requirements.txt",
+ }
+
+ var cwd util.AbsolutePath
+ // This bundle path does have requirements.txt file butis not included in configuration
+ bundleTestPath := cwd.Join("testdata", "python-bundle")
+
+ client := &ConnectClient{
+ client: httpClient,
+ }
+
+ err := client.CheckCapabilities(bundleTestPath, cfg, lgr)
+ s.NotNil(err)
+ s.Contains(err.Error(), "Missing dependency file requirements.txt. This file must be included in the deployment.")
aerr, yes := types.IsAgentError(err)
s.Equal(yes, true)
s.Equal(aerr.Code, types.ErrorRequirementsFileReading)
- s.Contains(aerr.Message, "Missing package file requirements.txt. The file must exist and be included in the deployment.")
+ s.Contains(aerr.Message, "Missing dependency file requirements.txt. This file must be included in the deployment.")
}
diff --git a/internal/publish/r_package_descriptions.go b/internal/publish/r_package_descriptions.go
index ee195b3cf..5515bf61e 100644
--- a/internal/publish/r_package_descriptions.go
+++ b/internal/publish/r_package_descriptions.go
@@ -21,11 +21,7 @@ type lockfileErrDetails struct {
Lockfile string
}
-const lockfileMissing = `Missing R lockfile %s.
-The file must exist and be included in the deployment.
-Create the file listing your project dependencies.
-Or do an automatic scan with the help of the R Packages section
-of the Publisher extension and review the generated file`
+const lockfileMissing = `Missing dependency lockfile %s. This file must be included in the deployment.`
func (p *defaultPublisher) getRPackages() (bundles.PackageMap, error) {
op := events.PublishGetRPackageDescriptionsOp
diff --git a/internal/publish/r_package_descriptions_test.go b/internal/publish/r_package_descriptions_test.go
index 5cde29c37..20dd7421b 100644
--- a/internal/publish/r_package_descriptions_test.go
+++ b/internal/publish/r_package_descriptions_test.go
@@ -152,7 +152,6 @@ func (s *RPackageDescSuite) TestGetRPackages_PackageFileNotFound() {
publisher := s.makePublisher()
_, err := publisher.getRPackages()
s.NotNil(err)
- s.Contains(err.(*types.AgentError).Message, "Missing R lockfile custom.lock")
- s.Contains(err.(*types.AgentError).Message, "custom.lock")
+ s.Contains(err.(*types.AgentError).Message, "Missing dependency lockfile custom.lock")
s.log.AssertExpectations(s.T())
}
From 4d06aef8b3d3876b73e6993fa014fde2ac49fbcd Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Fri, 11 Oct 2024 13:21:42 -0600
Subject: [PATCH 5/9] Fix lint on err message
---
internal/clients/connect/capabilities.go | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/internal/clients/connect/capabilities.go b/internal/clients/connect/capabilities.go
index 28a6d539d..edaa171a0 100644
--- a/internal/clients/connect/capabilities.go
+++ b/internal/clients/connect/capabilities.go
@@ -51,8 +51,10 @@ func checkRequirementsFile(base util.AbsolutePath, cfg *config.Config) error {
}
if !exists || !requirementsIsIncluded {
- missingErr := fmt.Errorf(requirementsFileMissing, cfg.Python.PackageFile)
- return types.NewAgentError(types.ErrorRequirementsFileReading, missingErr, requirementsErrDetails{RequirementsFile: packageFile.String()})
+ missingErr := fmt.Errorf("missing dependency file %s", cfg.Python.PackageFile)
+ aerr := types.NewAgentError(types.ErrorRequirementsFileReading, missingErr, requirementsErrDetails{RequirementsFile: packageFile.String()})
+ aerr.Message = fmt.Sprintf(requirementsFileMissing, cfg.Python.PackageFile)
+ return aerr
}
return nil
}
From 7a4f2f392b9eea2bbb6ea9ff2d896e6013228e99 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Fri, 11 Oct 2024 13:34:18 -0600
Subject: [PATCH 6/9] Fix deploy e2e test
---
test/bats/contract/deploy.bats | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/test/bats/contract/deploy.bats b/test/bats/contract/deploy.bats
index 284d19888..fccd0d194 100644
--- a/test/bats/contract/deploy.bats
+++ b/test/bats/contract/deploy.bats
@@ -139,10 +139,7 @@ python_content_types=(
run ${EXE} deploy ${FULL_PATH}
assert_failure
assert_output --partial "\
-Missing package file requirements.txt. The file must exist and be included in the deployment.
-Create the file listing your project dependencies.
-Or do an automatic scan with the help of the Python Packages section
-of the Publisher extension and review the generated file"
+Missing dependency file requirements.txt. This file must be included in the deployment."
else
skip
fi
From c67ffdbc271dc88be16c1937c3f2b9a40b3fb716 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Thu, 10 Oct 2024 15:21:40 -0600
Subject: [PATCH 7/9] Presentational adjustments to messaging when deployed
content does not run
---
extensions/vscode/src/api/types/error.test.ts | 96 +++++++++++++++++++
extensions/vscode/src/api/types/error.ts | 21 +++-
.../src/components/EvenEasierDeploy.vue | 29 +++++-
3 files changed, 139 insertions(+), 7 deletions(-)
create mode 100644 extensions/vscode/src/api/types/error.test.ts
diff --git a/extensions/vscode/src/api/types/error.test.ts b/extensions/vscode/src/api/types/error.test.ts
new file mode 100644
index 000000000..3d29ad91c
--- /dev/null
+++ b/extensions/vscode/src/api/types/error.test.ts
@@ -0,0 +1,96 @@
+// Copyright (C) 2024 by Posit Software, PBC.
+
+import { describe, expect, test } from "vitest";
+import {
+ isAgentErrorTypeUnknown,
+ isAgentErrorInvalidTOML,
+ isAgentErrorDeployedContentNotRunning,
+} from "./error";
+
+describe("Agent Errors", () => {
+ test("isAgentErrorTypeUnknown", () => {
+ let result = isAgentErrorTypeUnknown({
+ code: "unknown",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(true);
+
+ result = isAgentErrorTypeUnknown({
+ code: "resourceNotFound",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(true);
+
+ result = isAgentErrorTypeUnknown({
+ code: "unknownTOMLKey",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(false);
+
+ result = isAgentErrorTypeUnknown({
+ code: "invalidTOML",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(false);
+
+ result = isAgentErrorTypeUnknown({
+ code: "deployedContentNotRunning",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(false);
+ });
+
+ test("isAgentErrorInvalidTOML", () => {
+ let result = isAgentErrorInvalidTOML({
+ code: "unknown",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(false);
+
+ result = isAgentErrorInvalidTOML({
+ code: "unknownTOMLKey",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(true);
+
+ result = isAgentErrorInvalidTOML({
+ code: "invalidTOML",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(true);
+ });
+
+ test("isAgentErrorDeployedContentNotRunning", () => {
+ let result = isAgentErrorDeployedContentNotRunning({
+ code: "unknown",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(false);
+
+ result = isAgentErrorDeployedContentNotRunning({
+ code: "deployedContentNotRunning",
+ msg: "",
+ operation: "",
+ data: {},
+ });
+ expect(result).toBe(true);
+ });
+});
diff --git a/extensions/vscode/src/api/types/error.ts b/extensions/vscode/src/api/types/error.ts
index 32e2266c7..9e992d3c0 100644
--- a/extensions/vscode/src/api/types/error.ts
+++ b/extensions/vscode/src/api/types/error.ts
@@ -1,12 +1,17 @@
// Copyright (C) 2023 by Posit Software, PBC.
+import { ErrorCode } from "../../utils/errorTypes";
+
type AgentErrorBase = {
- code: string;
+ code: ErrorCode;
msg: string;
operation: string;
};
-export type AgentError = AgentErrorTypeUnknown | AgentErrorInvalidTOML;
+export type AgentError =
+ | AgentErrorTypeUnknown
+ | AgentErrorInvalidTOML
+ | AgentErrorContentNotRunning;
export type AgentErrorTypeUnknown = AgentErrorBase & {
data: {
@@ -19,7 +24,9 @@ export type AgentErrorTypeUnknown = AgentErrorBase & {
export const isAgentErrorTypeUnknown = (
e: AgentError,
): e is AgentErrorTypeUnknown => {
- return !isAgentErrorInvalidTOML(e);
+ return (
+ !isAgentErrorInvalidTOML(e) && !isAgentErrorDeployedContentNotRunning(e)
+ );
};
export type AgentErrorInvalidTOML = AgentErrorBase & {
@@ -36,3 +43,11 @@ export const isAgentErrorInvalidTOML = (
): e is AgentErrorInvalidTOML => {
return e.code === "invalidTOML" || e.code === "unknownTOMLKey";
};
+
+export type AgentErrorContentNotRunning = AgentErrorBase;
+
+export const isAgentErrorDeployedContentNotRunning = (
+ e: AgentError,
+): e is AgentErrorContentNotRunning => {
+ return e.code === "deployedContentNotRunning";
+};
diff --git a/extensions/vscode/webviews/homeView/src/components/EvenEasierDeploy.vue b/extensions/vscode/webviews/homeView/src/components/EvenEasierDeploy.vue
index 08d76933d..6aa432dc3 100644
--- a/extensions/vscode/webviews/homeView/src/components/EvenEasierDeploy.vue
+++ b/extensions/vscode/webviews/homeView/src/components/EvenEasierDeploy.vue
@@ -174,7 +174,7 @@
v-if="home.selectedContentRecord.deploymentError"
class="last-deployment-details last-deployment-error"
>
- <span class="codicon codicon-error error-icon"></span>
+ <span class="codicon codicon-alert"></span>
<TextStringWithAnchor
:message="home.selectedContentRecord?.deploymentError?.msg"
:splitOptions="ErrorMessageSplitOptions"
@@ -191,7 +191,7 @@
@click="viewContent"
class="w-full"
>
- View Content
+ {{ deployedContentButtonLabel }}
</vscode-button>
</div>
</div>
@@ -235,6 +235,7 @@ import TextStringWithAnchor from "./TextStringWithAnchor.vue";
import {
AgentError,
isAgentErrorInvalidTOML,
+ isAgentErrorDeployedContentNotRunning,
} from "../../../../src/api/types/error";
const home = useHomeStore();
@@ -438,6 +439,20 @@ const getActiveConfigTOMLErrorDetails = computed(() => {
return "";
});
+const isDeployedContentOnError = computed((): boolean => {
+ const deploymentError = home.selectedContentRecord?.deploymentError;
+ return Boolean(
+ deploymentError && isAgentErrorDeployedContentNotRunning(deploymentError),
+ );
+});
+
+const deployedContentButtonLabel = computed((): string => {
+ if (isDeployedContentOnError.value) {
+ return "View Deployment Logs";
+ }
+ return "View Content";
+});
+
const onEditConfigurationWithTOMLError = () => {
const agentError = getActiveConfigError.value;
if (agentError && isAgentErrorInvalidTOML(agentError)) {
@@ -492,8 +507,14 @@ const onAssociateDeployment = () => {
};
const viewContent = () => {
- if (home.selectedContentRecord?.dashboardUrl) {
- navigateToUrl(home.selectedContentRecord.dashboardUrl);
+ const record = home.selectedContentRecord;
+ if (!record) {
+ return;
+ }
+ if (isDeployedContentOnError.value && !isPreContentRecord(record)) {
+ navigateToUrl(record.logsUrl);
+ } else if (record.dashboardUrl) {
+ navigateToUrl(record.dashboardUrl);
}
};
</script>
From a7429cb6124bc786dfabd1d6faa8a3cac7c9b8a7 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Fri, 11 Oct 2024 14:36:51 -0600
Subject: [PATCH 8/9] AgentError messages punctuation helper
---
internal/clients/connect/capabilities.go | 5 +-
internal/clients/connect/client_connect.go | 4 +-
.../clients/connect/client_connect_test.go | 2 +-
internal/services/api/get_deployment_test.go | 2 +-
.../api/post_test_credentials_test.go | 2 +-
internal/types/error.go | 18 +++++++-
internal/types/error_test.go | 46 ++++++++++++++++++-
7 files changed, 68 insertions(+), 11 deletions(-)
diff --git a/internal/clients/connect/capabilities.go b/internal/clients/connect/capabilities.go
index edaa171a0..6384640ef 100644
--- a/internal/clients/connect/capabilities.go
+++ b/internal/clients/connect/capabilities.go
@@ -25,7 +25,7 @@ type allSettings struct {
quarto server_settings.QuartoInfo
}
-const requirementsFileMissing = `Missing dependency file %s. This file must be included in the deployment.`
+const requirementsFileMissing = `missing dependency file %s. This file must be included in the deployment`
type requirementsErrDetails struct {
RequirementsFile string `json:"requirements_file"`
@@ -51,9 +51,8 @@ func checkRequirementsFile(base util.AbsolutePath, cfg *config.Config) error {
}
if !exists || !requirementsIsIncluded {
- missingErr := fmt.Errorf("missing dependency file %s", cfg.Python.PackageFile)
+ missingErr := fmt.Errorf(requirementsFileMissing, cfg.Python.PackageFile)
aerr := types.NewAgentError(types.ErrorRequirementsFileReading, missingErr, requirementsErrDetails{RequirementsFile: packageFile.String()})
- aerr.Message = fmt.Sprintf(requirementsFileMissing, cfg.Python.PackageFile)
return aerr
}
return nil
diff --git a/internal/clients/connect/client_connect.go b/internal/clients/connect/client_connect.go
index db079bba1..1bfe629ab 100644
--- a/internal/clients/connect/client_connect.go
+++ b/internal/clients/connect/client_connect.go
@@ -492,9 +492,9 @@ func (c *ConnectClient) preflightAgentError(agenterr *types.AgentError, contentI
agenterr.Code = events.DeploymentFailedCode
if _, isNotFound := http_client.IsHTTPAgentErrorStatusOf(agenterr, http.StatusNotFound); isNotFound {
- agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Content cannot be found", contentID)
+ agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Content cannot be found.", contentID)
} else if _, isForbidden := http_client.IsHTTPAgentErrorStatusOf(agenterr, http.StatusForbidden); isForbidden {
- agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - You may need to request collaborator permissions or verify the credentials in use", contentID)
+ agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - You may need to request collaborator permissions or verify the credentials in use.", contentID)
} else {
agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Unknown error: %s", contentID, agenterr.Error())
}
diff --git a/internal/clients/connect/client_connect_test.go b/internal/clients/connect/client_connect_test.go
index 4cf743c93..1fe13e4aa 100644
--- a/internal/clients/connect/client_connect_test.go
+++ b/internal/clients/connect/client_connect_test.go
@@ -668,7 +668,7 @@ func (s *ConnectClientSuite) TestValidateDeploymentTargetNotFoundFailure() {
}
expectedErr := types.NewAgentError(
events.DeploymentFailedCode,
- errors.New("Cannot deploy content: ID e8922765-4880-43cd-abc0-d59fe59b8b4b - Content cannot be found"),
+ errors.New("cannot deploy content: ID e8922765-4880-43cd-abc0-d59fe59b8b4b - Content cannot be found"),
nil)
err := client.ValidateDeploymentTarget("e8922765-4880-43cd-abc0-d59fe59b8b4b", s.cfg, lgr)
aerr, ok := types.IsAgentError(err)
diff --git a/internal/services/api/get_deployment_test.go b/internal/services/api/get_deployment_test.go
index 22056a27f..de7ccdb1e 100644
--- a/internal/services/api/get_deployment_test.go
+++ b/internal/services/api/get_deployment_test.go
@@ -138,7 +138,7 @@ func (s *GetDeploymentSuite) TestGetPreDeployment() {
dec.DisallowUnknownFields()
s.NoError(dec.Decode(&res))
s.NotNil(res.Error)
- s.Equal("test error", res.Error.Message)
+ s.Equal("Test error.", res.Error.Message)
s.Equal(deploymentStateNew, res.State)
s.Equal("myTargetName", res.Name)
s.Equal(".", res.ProjectDir)
diff --git a/internal/services/api/post_test_credentials_test.go b/internal/services/api/post_test_credentials_test.go
index 6eee08636..72a6d254c 100644
--- a/internal/services/api/post_test_credentials_test.go
+++ b/internal/services/api/post_test_credentials_test.go
@@ -124,5 +124,5 @@ func (s *PostTestCredentialsHandlerSuite) TestPostTestCredentialsHandlerFuncBadA
s.NoError(err)
s.Nil(response.User)
s.NotNil(response.Error)
- s.Equal("test error from TestAuthentication", response.Error.Message)
+ s.Equal("Test error from TestAuthentication.", response.Error.Message)
}
diff --git a/internal/types/error.go b/internal/types/error.go
index 4ddd13db5..c8eaa4ff9 100644
--- a/internal/types/error.go
+++ b/internal/types/error.go
@@ -3,6 +3,9 @@ package types
// Copyright (C) 2023 by Posit Software, PBC.
import (
+ "slices"
+ "strings"
+
"github.com/mitchellh/mapstructure"
)
@@ -39,6 +42,19 @@ type AgentError struct {
Data ErrorData `json:"data" toml:"data,omitempty"`
}
+// Normalize punctuation on messages derived from errors
+func normalizeAgentErrorMsg(errMsg string) string {
+ spChars := []string{"?", "!", ")", "."}
+ msg := strings.ToUpper(errMsg[:1]) + errMsg[1:]
+
+ msgLastChar := msg[len(msg)-1:]
+ if slices.Contains(spChars, msgLastChar) {
+ return msg
+ }
+
+ return msg + "."
+}
+
func AsAgentError(e error) *AgentError {
if e == nil {
return nil
@@ -55,7 +71,7 @@ func NewAgentError(code ErrorCode, err error, details any) *AgentError {
msg := ""
if err != nil {
- msg = err.Error()
+ msg = normalizeAgentErrorMsg(err.Error())
}
if details != nil {
detailMap := make(ErrorData)
diff --git a/internal/types/error_test.go b/internal/types/error_test.go
index 0b4de2011..9dbf55f3f 100644
--- a/internal/types/error_test.go
+++ b/internal/types/error_test.go
@@ -61,7 +61,7 @@ func (s *ErrorSuite) TestNewAgentError() {
originalError := errors.New("shattered glass!")
aerr := NewAgentError(ErrorInvalidTOML, originalError, nil)
s.Equal(aerr, &AgentError{
- Message: "shattered glass!",
+ Message: "Shattered glass!",
Code: ErrorInvalidTOML,
Err: originalError,
Data: make(ErrorData),
@@ -78,9 +78,51 @@ func (s *ErrorSuite) TestIsAgentError() {
aerr, isIt = IsAgentError(aTrueAgentError)
s.Equal(isIt, true)
s.Equal(aerr, &AgentError{
- Message: "shattered glass!",
+ Message: "Shattered glass!",
Code: ErrorInvalidTOML,
Err: originalError,
Data: make(ErrorData),
})
}
+
+func (s *ErrorSuite) TestNewAgentError_MessagePunctuation() {
+ // Sentence case and period ending
+ originalError := errors.New("oh sorry, my mistake")
+ aerr := NewAgentError(ErrorResourceNotFound, originalError, nil)
+ s.Equal(aerr, &AgentError{
+ Message: "Oh sorry, my mistake.",
+ Code: ErrorResourceNotFound,
+ Err: originalError,
+ Data: make(ErrorData),
+ })
+
+ // No need for period ending when shouting "!"
+ originalError = errors.New("i can't believe is October already!")
+ aerr = NewAgentError(ErrorResourceNotFound, originalError, nil)
+ s.Equal(aerr, &AgentError{
+ Message: "I can't believe is October already!",
+ Code: ErrorResourceNotFound,
+ Err: originalError,
+ Data: make(ErrorData),
+ })
+
+ // No need for period ending when asking "?"
+ originalError = errors.New("can you believe 2024 is almos over?")
+ aerr = NewAgentError(ErrorResourceNotFound, originalError, nil)
+ s.Equal(aerr, &AgentError{
+ Message: "Can you believe 2024 is almos over?",
+ Code: ErrorResourceNotFound,
+ Err: originalError,
+ Data: make(ErrorData),
+ })
+
+ // No need for period ending when ending parentheses ")"
+ originalError = errors.New("wrong credentials (is this one of those bad typing days?)")
+ aerr = NewAgentError(ErrorResourceNotFound, originalError, nil)
+ s.Equal(aerr, &AgentError{
+ Message: "Wrong credentials (is this one of those bad typing days?)",
+ Code: ErrorResourceNotFound,
+ Err: originalError,
+ Data: make(ErrorData),
+ })
+}
From c91cdc5be3b9c0cd848454b21a240ff114839bb1 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Fri, 11 Oct 2024 15:16:49 -0600
Subject: [PATCH 9/9] Fix punctuation message test
---
internal/publish/publish_test.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go
index 4dd368ddc..ee0d248ee 100644
--- a/internal/publish/publish_test.go
+++ b/internal/publish/publish_test.go
@@ -207,7 +207,7 @@ func (s *PublishSuite) TestPublishWithClientRedeployErrors() {
s.publishWithClient(target, &publishErrsMock{capErr: checksErr}, checksErr)
s.Equal(
checksErr.Message,
- "failed",
+ "Failed.",
)
}
@@ -418,7 +418,7 @@ func (s *PublishSuite) TestEmitErrorEventsNoTarget() {
s.Len(emitter.Events, 2)
for _, event := range emitter.Events {
s.True(strings.HasSuffix(event.Type, "/failure"))
- s.Equal(expectedErr.Error(), event.Data["message"])
+ s.Equal("Test error.", event.Data["message"])
}
s.Equal("publish/failure", emitter.Events[1].Type)
}
@@ -454,7 +454,7 @@ func (s *PublishSuite) TestEmitErrorEventsWithTarget() {
s.Len(emitter.Events, 2)
for _, event := range emitter.Events {
s.True(strings.HasSuffix(event.Type, "/failure"))
- s.Equal(expectedErr.Error(), event.Data["message"])
+ s.Equal("Test error.", event.Data["message"])
s.Equal(util.GetDashboardURL("connect.example.com", targetID), event.Data["dashboardUrl"])
s.Equal(util.GetDirectURL("connect.example.com", targetID), event.Data["url"])
s.Equal(util.GetLogsURL("connect.example.com", targetID), event.Data["logsUrl"])