You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2017/12/01 01:20:40 UTC

[GitHub] csantanapr closed pull request #649: Support custom errors that pass in lower-level errors.

csantanapr closed pull request #649: Support custom errors that pass in lower-level errors.
URL: https://github.com/apache/incubator-openwhisk-wskdeploy/pull/649
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/deployers/deploymentreader.go b/deployers/deploymentreader.go
index af99451..39dbd76 100644
--- a/deployers/deploymentreader.go
+++ b/deployers/deploymentreader.go
@@ -18,6 +18,7 @@
 package deployers
 
 import (
+	"errors"
 	"github.com/apache/incubator-openwhisk-client-go/whisk"
 	"github.com/apache/incubator-openwhisk-wskdeploy/parsers"
 	"github.com/apache/incubator-openwhisk-wskdeploy/utils"
@@ -144,9 +145,10 @@ func (reader *DeploymentReader) bindPackageInputsAndAnnotations() error {
 					}
 				}
 				if !keyExistsInManifest {
-					err := wski18n.T("Annotation key \"" + name + "\" does not exist in manifest file but specified in deployment file.")
-					// TODO see if we can pass in the YAML file path on first parameter
-					return utils.NewYAMLFileFormatError(utils.LINE_UNKNOWN, err)
+					// TODO() i18n, need to use an ID
+					// TODO() fix grammar error; need command before "but"
+					err := errors.New(wski18n.T("Annotation key \"" + name + "\" does not exist in manifest file but specified in deployment file."))
+					return utils.NewYAMLFileFormatError(reader.DeploymentDescriptor.Filepath, err)
 				}
 			}
 		}
@@ -234,9 +236,9 @@ func (reader *DeploymentReader) bindActionInputsAndAnnotations() error {
 						}
 					}
 					if !keyExistsInManifest {
-						err := wski18n.T("Annotation key \"" + name + "\" does not exist in manifest file but specified in deployment file.")
-						// TODO see if we can pass in the YAML file path on first parameter
-						return utils.NewYAMLFileFormatError(utils.LINE_UNKNOWN, err)
+						// TODO() i18n, need to use an ID
+						err := errors.New(wski18n.T("Annotation key \"" + name + "\" does not exist in manifest file but specified in deployment file."))
+						return utils.NewYAMLFileFormatError(reader.DeploymentDescriptor.Filepath, err)
 					}
 				}
 			}
@@ -321,9 +323,9 @@ func (reader *DeploymentReader) bindTriggerInputsAndAnnotations() error {
 						}
 					}
 					if !keyExistsInManifest {
-						err := wski18n.T("Annotation key \"" + name + "\" does not exist in manifest file but specified in deployment file.")
-						// TODO see if we can pass in the YAML file path on first parameter
-						return utils.NewYAMLFileFormatError(utils.LINE_UNKNOWN, err)
+						// TODO() i18n, need to use an ID
+						err := errors.New(wski18n.T("Annotation key \"" + name + "\" does not exist in manifest file but specified in deployment file."))
+						return utils.NewYAMLFileFormatError(reader.DeploymentDescriptor.Filepath, err)
 					}
 				}
 			}
diff --git a/deployers/manifestreader.go b/deployers/manifestreader.go
index cf6ced1..85ece58 100644
--- a/deployers/manifestreader.go
+++ b/deployers/manifestreader.go
@@ -46,7 +46,7 @@ func (deployer *ManifestReader) ParseManifest() (*parsers.YAML, *parsers.YAMLPar
 	manifest, err := manifestParser.ParseManifest(dep.ManifestPath)
 
 	if err != nil {
-		return manifest, manifestParser, utils.NewFileReadError(dep.ManifestPath, err.Error())
+		return manifest, manifestParser, err
 	}
 	return manifest, manifestParser, nil
 }
@@ -54,7 +54,7 @@ func (deployer *ManifestReader) ParseManifest() (*parsers.YAML, *parsers.YAMLPar
 func (reader *ManifestReader) InitRootPackage(manifestParser *parsers.YAMLParser, manifest *parsers.YAML, ma whisk.KeyValue) error {
 	packages, err := manifestParser.ComposeAllPackages(manifest, reader.serviceDeployer.ManifestPath, ma)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifest.Filepath, err.Error())
+		return err
 	}
 	reader.SetPackage(packages)
 
@@ -69,66 +69,65 @@ func (deployer *ManifestReader) HandleYaml(sdeployer *ServiceDeployer, manifestP
 
 	deps, err := manifestParser.ComposeDependenciesFromAllPackages(manifest, deployer.serviceDeployer.ProjectPath, deployer.serviceDeployer.ManifestPath)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	actions, err := manifestParser.ComposeActionsFromAllPackages(manifest, deployer.serviceDeployer.ManifestPath, ma)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	sequences, err := manifestParser.ComposeSequencesFromAllPackages(deployer.serviceDeployer.ClientConfig.Namespace, manifest, ma)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	triggers, err := manifestParser.ComposeTriggersFromAllPackages(manifest, deployer.serviceDeployer.ManifestPath, ma)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	rules, err := manifestParser.ComposeRulesFromAllPackages(manifest)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	apis, err := manifestParser.ComposeApiRecordsFromAllPackages(manifest)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	err = deployer.SetDependencies(deps)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	err = deployer.SetActions(actions)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	err = deployer.SetSequences(sequences)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	err = deployer.SetTriggers(triggers)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	err = deployer.SetRules(rules)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	err = deployer.SetApis(apis)
 	if err != nil {
-		return utils.NewYAMLFileFormatError(manifestName, err.Error())
+		return utils.NewYAMLFileFormatError(manifestName, err)
 	}
 
 	return nil
-
 }
 
 func (reader *ManifestReader) SetDependencies(deps map[string]utils.DependencyRecord) error {
@@ -144,7 +143,7 @@ func (reader *ManifestReader) SetDependencies(deps map[string]utils.DependencyRe
 				gitReader := utils.NewGitReader(depName, dep)
 				err := gitReader.CloneDependency()
 				if err != nil {
-					return utils.NewYAMLFileFormatError(depName, err.Error())
+					return utils.NewYAMLFileFormatError(depName, err)
 				}
 			} else {
 				// TODO: we should do a check to make sure this dependency is compatible with an already installed one.
@@ -182,7 +181,10 @@ func (reader *ManifestReader) SetPackage(packages map[string]*whisk.Package) err
 				dep.Deployment.Packages[pkg.Name].Package = existPkg
 				return nil
 			} else {
-				return errors.New("Package " + pkg.Name + "exists twice")
+				// TODO(): i18n of error message (or create a new named error)
+				// TODO(): Is there a better way to handle an existing dependency of same name?
+				err := errors.New("Package [" + pkg.Name + "] exists already.")
+				return utils.NewYAMLParserErr(reader.serviceDeployer.ManifestPath, err)
 			}
 		}
 		newPack := NewDeploymentPackage()
@@ -227,18 +229,22 @@ func (reader *ManifestReader) SetActions(actions []utils.ActionRecord) error {
 
 				err := reader.checkAction(existAction)
 				if err != nil {
-					return utils.NewFileReadError(manifestAction.Filepath, err.Error())
+					return utils.NewFileReadError(manifestAction.Filepath, err)
 				}
 
 			} else {
 				// Action exists, but references two different sources
-				return errors.New("manifestReader. Error: Conflict detected for action named " + existAction.Action.Name + ". Found two locations for source file: " + existAction.Filepath + " and " + manifestAction.Filepath)
+				// TODO(): i18n of error message (or create a new named error)
+				err := errors.New("Conflict detected for action named [" +
+					existAction.Action.Name + "].\nFound two locations for source file: [" +
+					existAction.Filepath + "] and [" + manifestAction.Filepath + "]")
+				return utils.NewYAMLParserErr(reader.serviceDeployer.ManifestPath, err)
 			}
 		} else {
 			// not a new action so update the action in the package
 			err := reader.checkAction(manifestAction)
 			if err != nil {
-				return utils.NewFileReadError(manifestAction.Filepath, err.Error())
+				return utils.NewFileReadError(manifestAction.Filepath, err)
 			}
 			reader.serviceDeployer.Deployment.Packages[manifestAction.Packagename].Actions[manifestAction.Action.Name] = manifestAction
 		}
@@ -250,17 +256,23 @@ func (reader *ManifestReader) SetActions(actions []utils.ActionRecord) error {
 // TODO create named errors
 func (reader *ManifestReader) checkAction(action utils.ActionRecord) error {
 	if action.Filepath == "" {
-		return errors.New("Error: Action " + action.Action.Name + " has no source code location set.")
+		// TODO(): i18n of error message (or create a new named error)
+		err := errors.New("Action [" + action.Action.Name + "] has no source code location set.")
+		return utils.NewYAMLParserErr(reader.serviceDeployer.ManifestPath, err)
 	}
 
 	if action.Action.Exec.Kind == "" {
-		return errors.New("Error: Action " + action.Action.Name + " has no kind set")
+		// TODO(): i18n of error message (or create a new named error)
+		err := errors.New("Action [" + action.Action.Name + "] has no kind set.")
+		return utils.NewYAMLParserErr(reader.serviceDeployer.ManifestPath, err)
 	}
 
 	if action.Action.Exec.Code != nil {
 		code := *action.Action.Exec.Code
 		if code == "" && action.Action.Exec.Kind != "sequence" {
-			return errors.New("Error: Action " + action.Action.Name + " has no source code")
+			// TODO(): i18n of error message (or create a new named error)
+			err := errors.New("Action [" + action.Action.Name + "] has no source code.")
+			return utils.NewYAMLParserErr(reader.serviceDeployer.ManifestPath, err)
 		}
 	}
 
@@ -278,9 +290,10 @@ func (reader *ManifestReader) SetSequences(actions []utils.ActionRecord) error {
 		// If the sequence action exists in actions, return error
 		_, exists := reader.serviceDeployer.Deployment.Packages[seqAction.Packagename].Actions[seqAction.Action.Name]
 		if exists == true {
-			return errors.New("manifestReader. Error: Conflict sequence action with an action. " +
-				"Found a sequence action with the same name of an action:" +
-				seqAction.Action.Name)
+			// TODO(): i18n of error message (or create a new named error)
+			err := errors.New("Sequence action's name [" +
+				seqAction.Action.Name + "] is already used by an action.")
+			return utils.NewYAMLParserErr(reader.serviceDeployer.ManifestPath, err)
 		}
 		existAction, exists := reader.serviceDeployer.Deployment.Packages[seqAction.Packagename].Sequences[seqAction.Action.Name]
 
@@ -298,7 +311,7 @@ func (reader *ManifestReader) SetSequences(actions []utils.ActionRecord) error {
 			err := reader.checkAction(seqAction)
 			if err != nil {
 				// TODO() Need a better error type here
-				return utils.NewFileReadError(seqAction.Filepath, err.Error())
+				return utils.NewFileReadError(seqAction.Filepath, err)
 			}
 			reader.serviceDeployer.Deployment.Packages[seqAction.Packagename].Sequences[seqAction.Action.Name] = seqAction
 		}
diff --git a/deployers/servicedeployer.go b/deployers/servicedeployer.go
index 7bcb74d..9ce6ea4 100644
--- a/deployers/servicedeployer.go
+++ b/deployers/servicedeployer.go
@@ -132,16 +132,15 @@ func (deployer *ServiceDeployer) ConstructDeploymentPlan() error {
 		// OpenWhisk entities are annotated with Project Name and therefore
 		// Project Name in manifest/deployment file is mandatory for managed deployments
 		if deployer.ProjectName == "" {
-			// TODO see if we can pass in the Deployment file path on first parameter
 			// TODO see if we can move string to translation file.
-			return utils.NewYAMLFileFormatError(utils.LINE_UNKNOWN, "Project name in manifest file is mandatory for managed deployments")
+			return utils.NewYAMLFileFormatError(manifest.Filepath, "Project name in manifest file is mandatory for managed deployments")
 		}
 		// Every OpenWhisk entity in the manifest file will be annotated with:
 		//managed: '{"__OW__PROJECT__NAME": <name>, "__OW__PROJECT_HASH": <hash>, "__OW__FILE": <path>}'
 		deployer.ManagedAnnotation, err = utils.GenerateManagedAnnotation(deployer.ProjectName, manifest.Filepath)
 		if err != nil {
 			// TODO see if we can pass in the YAML file path on first parameter
-			return utils.NewYAMLFileFormatError(utils.LINE_UNKNOWN, err.Error())
+			return utils.NewYAMLFileFormatError(manifest.Filepath, err.Error())
 		}
 	}
 
@@ -1241,7 +1240,7 @@ func (deployer *ServiceDeployer) printDeploymentAssets(assets *DeploymentProject
 		for _, p := range pack.Package.Parameters {
 			jsonValue, err := utils.PrettyJSON(p.Value)
 			if err != nil {
-				fmt.Printf("        - %s : %s\n", p.Key, utils.UNKNOWN_VALUE)
+				fmt.Printf("        - %s : %s\n", p.Key, utils.STR_UNKNOWN_VALUE)
 			} else {
 				fmt.Printf("        - %s : %v\n", p.Key, jsonValue)
 			}
@@ -1269,7 +1268,7 @@ func (deployer *ServiceDeployer) printDeploymentAssets(assets *DeploymentProject
 					} else {
 						jsonValue, err := utils.PrettyJSON(p.Value)
 						if err != nil {
-							fmt.Printf("        - %s : %s\n", p.Key, utils.UNKNOWN_VALUE)
+							fmt.Printf("        - %s : %s\n", p.Key, utils.STR_UNKNOWN_VALUE)
 						} else {
 							fmt.Printf("        - %s : %v\n", p.Key, jsonValue)
 						}
@@ -1277,7 +1276,7 @@ func (deployer *ServiceDeployer) printDeploymentAssets(assets *DeploymentProject
 				} else {
 					jsonValue, err := utils.PrettyJSON(p.Value)
 					if err != nil {
-						fmt.Printf("        - %s : %s\n", p.Key, utils.UNKNOWN_VALUE)
+						fmt.Printf("        - %s : %s\n", p.Key, utils.STR_UNKNOWN_VALUE)
 					} else {
 						fmt.Printf("        - %s : %v\n", p.Key, jsonValue)
 					}
@@ -1307,7 +1306,7 @@ func (deployer *ServiceDeployer) printDeploymentAssets(assets *DeploymentProject
 		for _, p := range trigger.Parameters {
 			jsonValue, err := utils.PrettyJSON(p.Value)
 			if err != nil {
-				fmt.Printf("        - %s : %s\n", p.Key, utils.UNKNOWN_VALUE)
+				fmt.Printf("        - %s : %s\n", p.Key, utils.STR_UNKNOWN_VALUE)
 			} else {
 				fmt.Printf("        - %s : %v\n", p.Key, jsonValue)
 			}
diff --git a/parsers/deploy_parser.go b/parsers/deploy_parser.go
index a751737..081e5c4 100644
--- a/parsers/deploy_parser.go
+++ b/parsers/deploy_parser.go
@@ -20,7 +20,6 @@ package parsers
 import (
 	"github.com/apache/incubator-openwhisk-wskdeploy/utils"
 	"gopkg.in/yaml.v2"
-    "strings"
 )
 
 func (dm *YAMLParser) unmarshalDeployment(input []byte, deploy *YAML) error {
@@ -39,28 +38,13 @@ func (dm *YAMLParser) ParseDeployment(deploymentPath string) (*YAML, error) {
     }
 	err = dm.unmarshalDeployment(content, &dplyyaml)
     if err != nil {
-        lines, msgs := dm.convertErrorToLinesMsgs(err.Error())
-        return &dplyyaml, utils.NewYAMLParserErr(deploymentPath, lines, msgs)
+        return &dplyyaml, utils.NewYAMLParserErr(deploymentPath, err)
     }
 	dplyyaml.Filepath = deploymentPath
     dplyyamlEnvVar := ReadEnvVariable(&dplyyaml)
 	return dplyyamlEnvVar, nil
 }
 
-func (dm *YAMLParser) convertErrorToLinesMsgs(errorString string) (lines []string, msgs []string) {
-    strs := strings.Split(errorString, "\n")
-    for i := 0; i < len(strs); i++ {
-        var errorMsg string
-	if strings.Contains(strs[i], utils.LINE) {
-		errorMsg = strings.Replace(strs[i], utils.LINE, "(on or near) "+utils.LINE, 1)
-	} else {
-		errorMsg = strs[i]
-	}
-        lines = append(lines, utils.LINE_UNKNOWN)
-        msgs = append(msgs, strings.TrimSpace(errorMsg))
-    }
-    return
-}
 
 //********************Project functions*************************//
 //This is for parse the deployment yaml file.
diff --git a/parsers/manifest_parser.go b/parsers/manifest_parser.go
index 85bab5e..294d9b7 100644
--- a/parsers/manifest_parser.go
+++ b/parsers/manifest_parser.go
@@ -92,8 +92,7 @@ func (dm *YAMLParser) ParseManifest(manifestPath string) (*YAML, error) {
 
 	err = mm.Unmarshal(content, &maniyaml)
 	if err != nil {
-		lines, msgs := dm.convertErrorToLinesMsgs(err.Error())
-		return &maniyaml, utils.NewYAMLParserErr(manifestPath, lines, msgs)
+		return &maniyaml, utils.NewYAMLParserErr(manifestPath, err)
 	}
 	maniyaml.Filepath = manifestPath
 	manifest := ReadEnvVariable(&maniyaml)
diff --git a/parsers/manifest_parser_test.go b/parsers/manifest_parser_test.go
index cb79e1a..2f1d7fa 100644
--- a/parsers/manifest_parser_test.go
+++ b/parsers/manifest_parser_test.go
@@ -932,15 +932,11 @@ func TestResolveParameterForMultiLineParams(t *testing.T) {
     param5 := Parameter{Type: "invalid", multiline: true}
     _, err := ResolveParameter(p, &param5, "")
     assert.NotNil(t, err, "Expected error saying Invalid type for parameter")
-    lines := []string{"Line Unknown"}
-    msgs := []string{"Invalid Type for parameter. [invalid]"}
-    expectedErr := utils.NewYAMLParserErr("", lines, msgs)
     switch errorType := err.(type) {
     default:
         assert.Fail(t, "Wrong error type received: We are expecting ParserErr.")
     case *utils.YAMLParserError:
-        assert.Equal(t, expectedErr.Message, errorType.Message,
-            "Expected error " + expectedErr.Message + " but found " + errorType.Message)
+        assert.Equal(t, "Parameter [name] has an invalid Type. [invalid]", errorType.Message)
     }
 
     // type none - param without type, without value, and without default value
diff --git a/parsers/parameters.go b/parsers/parameters.go
index 1767ec8..24a389f 100644
--- a/parsers/parameters.go
+++ b/parsers/parameters.go
@@ -158,8 +158,9 @@ func resolveSingleLineParameter(filePath string, paramName string, param *Parame
 		}
 
 	} else {
-		msgs := []string{"Parameter [" + paramName + "] is not single-line format."}
-		return param.Value, utils.NewYAMLParserErr(filePath, nil, msgs)
+		// TODO() - move string to i18n
+		return param.Value, utils.NewYAMLParserErr(filePath,
+			"Parameter [" + paramName + "] is not single-line format.")
 	}
 
 	return param.Value, errorParser
@@ -199,8 +200,8 @@ func resolveMultiLineParameter(filePath string, paramName string, param *Paramet
 		if param.Type != "" {
 			if !isValidParameterType(param.Type) {
 				// TODO() - move string to i18n
-				msgs := []string{"Parameter [" + paramName + "] has an invalid Type. [" + param.Type + "]"}
-				return param.Value, utils.NewYAMLParserErr(filePath, nil, msgs)
+				return param.Value, utils.NewYAMLParserErr(filePath,
+					"Parameter [" + paramName + "] has an invalid Type. [" + param.Type + "]")
 			}
 		} else {
 			// if we do not have a value for the Parameter Type, use the Parameter Value's Type
@@ -212,8 +213,9 @@ func resolveMultiLineParameter(filePath string, paramName string, param *Paramet
 		//	errorParser = utils.NewParameterTypeMismatchError("", param.Type, valueType )
 		//}
 	} else {
-		msgs := []string{"Parameter [" + paramName + "] is not multiline format."}
-		return param.Value, utils.NewYAMLParserErr(filePath, nil, msgs)
+		// TODO() - move string to i18n
+		return param.Value, utils.NewYAMLParserErr(filePath,
+			"Parameter [" + paramName + "] is not multiline format.")
 	}
 
 
@@ -267,8 +269,8 @@ func resolveJSONParameter(filePath string, paramName string, param *Parameter, v
 		}
 
 	} else {
-		msgs := []string{"Parameter [" + paramName + "] is not JSON format."}
-		errorParser = utils.NewYAMLParserErr(filePath, nil, msgs)
+		// TODO() - move string to i18n
+		errorParser = utils.NewYAMLParserErr(filePath, "Parameter [" + paramName + "] is not JSON format.")
 	}
 
 	return param.Value, errorParser
diff --git a/tests/dat/manifest_bad_yaml_invalid_comment.yaml b/tests/dat/manifest_bad_yaml_invalid_comment.yaml
index e1330e5..1102197 100644
--- a/tests/dat/manifest_bad_yaml_invalid_comment.yaml
+++ b/tests/dat/manifest_bad_yaml_invalid_comment.yaml
@@ -10,5 +10,7 @@ packages:
             description: name of a person
           place:
             type: string
-            /// bad description
+            /// bad comment
             description: location of a person
+
+# go-yaml/yaml "yaml: line 13: could not find expected ':'"
diff --git a/tests/dat/manifest_bad_yaml_invalid_key_mapping_value.yaml b/tests/dat/manifest_bad_yaml_invalid_key_mapping_value.yaml
index 5d0cd70..659a2d0 100644
--- a/tests/dat/manifest_bad_yaml_invalid_key_mapping_value.yaml
+++ b/tests/dat/manifest_bad_yaml_invalid_key_mapping_value.yaml
@@ -5,4 +5,3 @@ packages:
       actions: test
 
 # go-yaml/yaml "line 4: mapping values are not allowed in this context".
-
diff --git a/tests/dat/manifest_validate_trigger_action_rule_grammar.yaml b/tests/dat/manifest_validate_trigger_action_rule_grammar.yaml
index ba024f1..64ed6b5 100644
--- a/tests/dat/manifest_validate_trigger_action_rule_grammar.yaml
+++ b/tests/dat/manifest_validate_trigger_action_rule_grammar.yaml
@@ -5,8 +5,11 @@ packages:
       license: Apache-2.0
       actions:
         first_action:
+          function: actions/dump_params.js
         second_action:
+          function: actions/dump_params.js
         third_action:
+          function: actions/dump_params.js
       triggers:
         trigger1:
         trigger2:
diff --git a/utils/wskdeployerror.go b/utils/wskdeployerror.go
index 1c7444b..d5175e8 100644
--- a/utils/wskdeployerror.go
+++ b/utils/wskdeployerror.go
@@ -25,16 +25,22 @@ import (
 )
 
 const (
-	LINE_UNKNOWN = "Unknown"
-	UNKNOWN_VALUE = "Unknown value"
-	FILE = "File"
-	LINE = "Line"
-	PARAMETER = "Parameter"
-	TYPE = "Type"
-	EXPECTED = "expected"
-	ACTUAL = "actual"
-	INDENT_LEVEL_1 = "==>"
-
+	// Error message compositional strings
+	STR_UNKNOWN_VALUE = "Unknown value"
+	STR_COMMAND = "Command"
+	STR_ERROR_CODE = "Error code"
+	STR_FILE = "File"
+	STR_LINE = "Line"
+	STR_PARAMETER = "Parameter"
+	STR_TYPE = "Type"
+	STR_EXPECTED = "Expected"
+	STR_ACTUAL = "Actual"
+	STR_NEWLINE = "\n"
+
+	// Formatting
+	STR_INDENT_1 = "==>"
+
+	// Error Types
 	ERROR_COMMAND_FAILED = "ERROR_COMMAND_FAILED"
 	ERROR_WHISK_CLIENT_ERROR = "ERROR_WHISK_CLIENT_ERROR"
 	ERROR_WHISK_CLIENT_INVALID_CONFIG = "ERROR_WHISK_CLIENT_INVALID_CONFIG"
@@ -49,35 +55,61 @@ const (
 /*
  * BaseError
  */
-type BaseErr struct {
+type WskDeployBaseErr struct {
 	ErrorType string
 	FileName  string
 	LineNum   int
 	Message   string
 }
 
-func (e *BaseErr) Error() string {
+func (e *WskDeployBaseErr) Error() string {
 	return fmt.Sprintf("%s [%d]: [%s]: %s\n", e.FileName, e.LineNum, e.ErrorType, e.Message)
 }
 
-func (e *BaseErr) SetFileName(fileName string) {
+func (e *WskDeployBaseErr) SetFileName(fileName string) {
 	e.FileName = filepath.Base(fileName)
 }
 
-func (e *BaseErr) SetLineNum(lineNum int) {
+func (e *WskDeployBaseErr) SetLineNum(lineNum int) {
 	e.LineNum = lineNum
 }
 
-func (e *BaseErr) SetMessage(message string) {
-	e.Message = message
+func (e *WskDeployBaseErr) SetErrorType(errorType string) {
+	e.ErrorType = errorType
 }
 
-func (e *BaseErr) SetErrorType(errorType string) {
-	e.ErrorType = errorType
+func (e *WskDeployBaseErr) SetMessage(message interface{}) {
+
+	if message != nil{
+		switch message.(type) {
+		case string:
+			e.Message = message.(string)
+		case error:
+			err := message.(error)
+			e.appendErrorDetails(err)
+		}
+	}
+}
+
+func (e *WskDeployBaseErr) appendDetail(detail string){
+	fmt := fmt.Sprintf("\n%s %s", STR_INDENT_1, detail)
+	e.Message = e.Message + fmt
+}
+
+func (e *WskDeployBaseErr) appendErrorDetails(err error){
+	if err != nil {
+		errorMsg := err.Error()
+		var detailMsg string
+		msgs := strings.Split(errorMsg, STR_NEWLINE)
+		for i := 0; i < len(msgs); i++ {
+			detailMsg = msgs[i]
+			e.appendDetail(strings.TrimSpace(detailMsg))
+		}
+	}
 }
 
 // func Caller(skip int) (pc uintptr, file string, line int, ok bool)
-func (e *BaseErr) SetCallerByStackFrameSkip(skip int) {
+func (e *WskDeployBaseErr) SetCallerByStackFrameSkip(skip int) {
 	_, fname, lineNum, _ := runtime.Caller(skip)
 	e.SetFileName(fname)
 	e.SetLineNum(lineNum)
@@ -87,7 +119,7 @@ func (e *BaseErr) SetCallerByStackFrameSkip(skip int) {
  * CommandError
  */
 type CommandError struct {
-	BaseErr
+	WskDeployBaseErr
 	Command string
 }
 
@@ -97,19 +129,16 @@ func NewCommandError(cmd string, errorMessage string) *CommandError {
 	}
 	err.SetErrorType(ERROR_COMMAND_FAILED)
 	err.SetCallerByStackFrameSkip(2)
-	err.SetMessage(cmd + ": " + errorMessage)
+	str := fmt.Sprintf("%s: [%s]: %s", STR_COMMAND, cmd, errorMessage)
+	err.SetMessage(str)
 	return err
 }
 
-func (e *CommandError) Error() string {
-	return e.BaseErr.Error()
-}
-
 /*
  * WhiskClientError
  */
 type WhiskClientError struct {
-	BaseErr
+	WskDeployBaseErr
 	ErrorCode int
 }
 
@@ -119,7 +148,7 @@ func NewWhiskClientError(errorMessage string, code int) *WhiskClientError {
 	}
 	err.SetErrorType(ERROR_WHISK_CLIENT_ERROR)
 	err.SetCallerByStackFrameSkip(2)
-	str := fmt.Sprintf("Error Code: %d: %s", code, errorMessage)
+	str := fmt.Sprintf("%s: %d: %s", STR_ERROR_CODE, code, errorMessage)
 	err.SetMessage(str)
 	return err
 }
@@ -128,7 +157,7 @@ func NewWhiskClientError(errorMessage string, code int) *WhiskClientError {
  * WhiskClientInvalidConfigError
  */
 type WhiskClientInvalidConfigError struct {
-	BaseErr
+	WskDeployBaseErr
 }
 
 func NewWhiskClientInvalidConfigError(errorMessage string) *WhiskClientInvalidConfigError {
@@ -144,7 +173,7 @@ func NewWhiskClientInvalidConfigError(errorMessage string) *WhiskClientInvalidCo
  * FileError
  */
 type FileError struct {
-	BaseErr
+	WskDeployBaseErr
 	ErrorFileName string
 	ErrorFilePath string
 }
@@ -159,7 +188,7 @@ func (e *FileError) SetErrorFileName(fname string) {
 }
 
 func (e *FileError) Error() string {
-	return fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s\n",
+	return fmt.Sprintf("%s [%d]: [%s]: " + STR_FILE + ": [%s]: %s\n",
 		e.FileName,
 		e.LineNum,
 		e.ErrorType,
@@ -170,8 +199,13 @@ func (e *FileError) Error() string {
 /*
  * FileReadError
  */
-func NewFileReadError(fpath string, errMessage string) *FileError {
-	var err = &FileError{
+
+type FileReadError struct {
+	FileError
+}
+
+func NewFileReadError(fpath string, errMessage interface{}) *FileReadError {
+	var err = &FileReadError{
 	}
 	err.SetErrorType(ERROR_FILE_READ_ERROR)
 	err.SetCallerByStackFrameSkip(2)
@@ -188,7 +222,7 @@ type ErrorManifestFileNotFound struct {
 	FileError
 }
 
-func NewErrorManifestFileNotFound(fpath string, errMessage string) *ErrorManifestFileNotFound {
+func NewErrorManifestFileNotFound(fpath string, errMessage interface{}) *ErrorManifestFileNotFound {
 	var err = &ErrorManifestFileNotFound{
 	}
 	err.SetErrorType(ERROR_MANIFEST_FILE_NOT_FOUND)
@@ -205,7 +239,7 @@ type YAMLFileFormatError struct {
 	FileError
 }
 
-func NewYAMLFileFormatError(fpath string, errorMessage string) *YAMLFileFormatError {
+func NewYAMLFileFormatError(fpath string, errorMessage interface{}) *YAMLFileFormatError {
 	var err = &YAMLFileFormatError{
 	}
 	err.SetErrorType(ERROR_YAML_FILE_FORMAT_ERROR)
@@ -235,10 +269,10 @@ func NewParameterTypeMismatchError(fpath string, param string, expectedType stri
 	err.SetCallerByStackFrameSkip(2)
 	err.SetErrorFilePath(fpath)
 	str := fmt.Sprintf("%s [%s]: %s %s: [%s], %s: [%s]",
-		PARAMETER, param,
-		TYPE,
-		EXPECTED, expectedType,
-		ACTUAL, actualType)
+		STR_PARAMETER, param,
+		STR_TYPE,
+		STR_EXPECTED, expectedType,
+		STR_ACTUAL, actualType)
 	err.SetMessage(str)
 	return err
 }
@@ -260,8 +294,8 @@ func NewInvalidParameterTypeError(fpath string, param string, actualType string)
 	err.SetErrorType(ERROR_YAML_INVALID_PARAMETER_TYPE)
 	err.SetCallerByStackFrameSkip(2)
 	str := fmt.Sprintf("%s [%s]: %s [%s]",
-		PARAMETER, param,
-		TYPE, actualType)
+		STR_PARAMETER, param,
+		STR_TYPE, actualType)
 	err.SetMessage(str)
 	return err
 }
@@ -275,31 +309,32 @@ type YAMLParserError struct {
 	msgs     []string
 }
 
-func NewYAMLParserErr(fpath string, lines []string, msgs []string) *YAMLParserError {
+func NewYAMLParserErr(fpath string, msg interface{}) *YAMLParserError {
 	var err = &YAMLParserError{
-		lines: lines,
-		msgs: msgs,
+
 	}
 	err.SetErrorType(ERROR_YAML_PARSER_ERROR)
 	err.SetErrorFilePath(fpath)
 	err.SetCallerByStackFrameSkip(2)
+        err.SetMessage(msg)
 	return err
 }
 
-
-func (e *YAMLParserError) Error() string {
-	result := make([]string, len(e.msgs))
-
-	for index, msg := range e.msgs {
-		var s string
-		if e.lines == nil || e.lines[index] == LINE_UNKNOWN {
-			s = fmt.Sprintf("\n%s %s [%v]: %s", INDENT_LEVEL_1, LINE, LINE_UNKNOWN, msg)
-		} else {
-			s = fmt.Sprintf("\n%s %s [%v]: %s", INDENT_LEVEL_1, LINE, e.lines[index], msg)
-		}
-		result[index] = s
+func IsCustomError( err error ) bool {
+
+	switch err.(type) {
+
+	case *CommandError:
+	case *WhiskClientError:
+	case *WhiskClientInvalidConfigError:
+	case *FileError:
+	case *FileReadError:
+	case *ErrorManifestFileNotFound:
+	case *YAMLFileFormatError:
+	case *ParameterTypeMismatchError:
+	case *InvalidParameterTypeError:
+	case *YAMLParserError:
+		return true
 	}
-
-	e.SetMessage(strings.Join(result, ""))
-	return e.FileError.Error()
+	return false
 }
diff --git a/utils/wskdeployerror_test.go b/utils/wskdeployerror_test.go
index 18c809e..f9a4cb6 100644
--- a/utils/wskdeployerror_test.go
+++ b/utils/wskdeployerror_test.go
@@ -51,26 +51,28 @@ func TestCustomErrorOutputFormat(t *testing.T) {
 	 */
 	err1 := NewCommandError(TEST_COMMAND, TEST_DEFAULT_ERROR_MESSAGE)
 	actualResult :=  strings.TrimSpace(err1.Error())
-	expectedResult := fmt.Sprintf("%s [%d]: [%s]: %s: %s",
+	expectedResult := fmt.Sprintf("%s [%d]: [%s]: %s: [%s]: %s",
 		packageName,
 		err1.LineNum,
 		ERROR_COMMAND_FAILED,
+		STR_COMMAND,
 		TEST_COMMAND,
 		TEST_DEFAULT_ERROR_MESSAGE )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
 	 * WhiskClientError
 	 */
 	err2 := NewWhiskClientError(TEST_DEFAULT_ERROR_MESSAGE, TEST_ERROR_CODE)
 	actualResult =  strings.TrimSpace(err2.Error())
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: Error Code: %d: %s",
+	expectedResult = fmt.Sprintf("%s [%d]: [%s]: %s: %d: %s",
 		packageName,
 		err2.LineNum,
 		ERROR_WHISK_CLIENT_ERROR,
+		STR_ERROR_CODE,
 		TEST_ERROR_CODE,
 		TEST_DEFAULT_ERROR_MESSAGE )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
 	 * WhiskClientInvalidConfigError
@@ -82,46 +84,48 @@ func TestCustomErrorOutputFormat(t *testing.T) {
 		err3.LineNum,
 		ERROR_WHISK_CLIENT_INVALID_CONFIG,
 		TEST_DEFAULT_ERROR_MESSAGE )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
  	 * FileReadError
  	 */
 	err4 := NewFileReadError(TEST_NONEXISTANT_MANIFEST_FILE, TEST_DEFAULT_ERROR_MESSAGE)
 	actualResult =  strings.TrimSpace(err4.Error())
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s",
+	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + STR_FILE + ": [%s]: %s",
 		packageName,
 		err4.LineNum,
 		ERROR_FILE_READ_ERROR,
 		filepath.Base(TEST_NONEXISTANT_MANIFEST_FILE),
 		TEST_DEFAULT_ERROR_MESSAGE )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
  	 * ManifestFileNotFoundError
  	 */
 	err5 := NewErrorManifestFileNotFound(TEST_NONEXISTANT_MANIFEST_FILE, TEST_DEFAULT_ERROR_MESSAGE)
 	actualResult =  strings.TrimSpace(err5.Error())
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s",
+	expectedResult = fmt.Sprintf("%s [%d]: [%s]: %s: [%s]: %s",
 		packageName,
 		err5.LineNum,
 		ERROR_MANIFEST_FILE_NOT_FOUND,
+		STR_FILE,
 		filepath.Base(TEST_NONEXISTANT_MANIFEST_FILE),
 		TEST_DEFAULT_ERROR_MESSAGE )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
          * YAMLFileFormatError
          */
 	err6 := NewYAMLFileFormatError(TEST_INVALID_YAML_MANIFEST_FILE, TEST_DEFAULT_ERROR_MESSAGE)
 	actualResult =  strings.TrimSpace(err6.Error())
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s",
+	expectedResult = fmt.Sprintf("%s [%d]: [%s]: %s: [%s]: %s",
 		packageName,
 		err6.LineNum,
 		ERROR_YAML_FILE_FORMAT_ERROR,
+		STR_FILE,
 		filepath.Base(TEST_INVALID_YAML_MANIFEST_FILE),
 		TEST_DEFAULT_ERROR_MESSAGE )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
 	 * ParameterTypeMismatchError
@@ -133,17 +137,18 @@ func TestCustomErrorOutputFormat(t *testing.T) {
 		TEST_PARAM_TYPE_FLOAT)
 	actualResult =  strings.TrimSpace(err8.Error())
 	msg8 := fmt.Sprintf("%s [%s]: %s %s: [%s], %s: [%s]",
-		PARAMETER, TEST_PARAM_NAME,
-		TYPE,
-		EXPECTED, TEST_PARAM_TYPE_INT,
-		ACTUAL, TEST_PARAM_TYPE_FLOAT)
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s",
+		STR_PARAMETER, TEST_PARAM_NAME,
+		STR_TYPE,
+		STR_EXPECTED, TEST_PARAM_TYPE_INT,
+		STR_ACTUAL, TEST_PARAM_TYPE_FLOAT)
+	expectedResult = fmt.Sprintf("%s [%d]: [%s]: %s: [%s]: %s",
 		packageName,
 		err8.LineNum,
 		ERROR_YAML_PARAMETER_TYPE_MISMATCH,
+		STR_FILE,
 		filepath.Base(TEST_EXISTANT_MANIFEST_FILE),
 		msg8 )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
 	 * InvalidParameterType
@@ -151,34 +156,43 @@ func TestCustomErrorOutputFormat(t *testing.T) {
 	err9 := NewInvalidParameterTypeError(TEST_EXISTANT_MANIFEST_FILE, TEST_PARAM_NAME, TEST_PARAM_TYPE_FOO)
 	actualResult =  strings.TrimSpace(err9.Error())
 	msg9 := fmt.Sprintf("%s [%s]: %s [%s]",
-		PARAMETER, TEST_PARAM_NAME,
-		TYPE, TEST_PARAM_TYPE_FOO)
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s",
+		STR_PARAMETER, TEST_PARAM_NAME,
+		STR_TYPE, TEST_PARAM_TYPE_FOO)
+	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + STR_FILE + ": [%s]: %s",
 		packageName,
 		err9.LineNum,
 		ERROR_YAML_INVALID_PARAMETER_TYPE,
 		filepath.Base(TEST_EXISTANT_MANIFEST_FILE),
 		msg9 )
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	assert.Equal(t, expectedResult, actualResult)
 
 	/*
 	 * YAMLParserErr
 	 */
-	var TEST_LINES    = []string{"40", LINE_UNKNOWN, "123"}
-	var TEST_MESSAGES = []string{"did not find expected key", "did not find expected ',' or ']'", "found duplicate %YAML directive"}
 
-	err10 := NewYAMLParserErr(TEST_EXISTANT_MANIFEST_FILE, TEST_LINES, TEST_MESSAGES)
-	actualResult =  strings.TrimSpace(err10.Error())
-
-	msgs := "\n==> Line [40]: did not find expected key" +
-		"\n==> Line [Unknown]: did not find expected ',' or ']'" +
-		"\n==> Line [123]: found duplicate %YAML directive"
-
-	expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + FILE + ": [%s]: %s",
-		packageName,
-		err10.LineNum,
-		ERROR_YAML_PARSER_ERROR,
-		filepath.Base(TEST_EXISTANT_MANIFEST_FILE),
-		msgs)
-	assert.Equal(t, expectedResult, actualResult, "Expected [" + expectedResult + "] but got [" + actualResult + "]")
+	// TODO add a unit test once we re-factor error related modules into a new package
+	// to avoid cyclic dependency errors in GoLang.
+	//manifestFile := "../tests/dat/manifest_bad_yaml_invalid_comment.yaml"
+	//// read and parse manifest.yaml file
+	//m, err := parsers.NewYAMLParser().ParseManifest(manifestFile)
+	//fmt.Println(err)
+
+	// TODO - use actual YAML files to generate actual errors for comparison with expected error output
+	//var TEST_LINES    = []string{"40", STR_UNKNOWN, "123"}
+	//var TEST_MESSAGES = []string{"did not find expected key", "did not find expected ',' or ']'", "found duplicate %YAML directive"}
+	//
+	//err10 := NewYAMLParserErr(TEST_EXISTANT_MANIFEST_FILE, TEST_LINES, TEST_MESSAGES)
+	//actualResult =  strings.TrimSpace(err10.Error())
+	//
+	//msgs := "\n==> Line [40]: did not find expected key" +
+	//	"\n==> Line [Unknown]: did not find expected ',' or ']'" +
+	//	"\n==> Line [123]: found duplicate %YAML directive"
+	//
+	//expectedResult = fmt.Sprintf("%s [%d]: [%s]: " + STR_FILE + ": [%s]: %s",
+	//	packageName,
+	//	err10.LineNum,
+	//	ERROR_YAML_PARSER_ERROR,
+	//	filepath.Base(TEST_EXISTANT_MANIFEST_FILE),
+	//	msgs)
+	//assert.Equal(t, expectedResult, actualResult)
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services