From 6ef96a1ecd7bc6d3bda013f3fee50928664d5712 Mon Sep 17 00:00:00 2001 From: Will Lahti Date: Wed, 18 Jan 2017 12:13:28 -0500 Subject: [PATCH] Simplify error handling framework messages This changeset removes the error handling framework's use of an error message map and any code that was in place to add future language translations. This simplification will allow easier adoption of internationalization packages whenever that is deemed a priority. https://jira.hyperledger.org/browse/FAB-1571 Change-Id: I0ae562ebdc10d43c99d5099039671769199af95e Signed-off-by: Will Lahti --- core/errors/errors.go | 57 ++++++++++--------------- core/errors/errors_json.go | 86 -------------------------------------- core/errors/errors_test.go | 40 ++++++++---------- peer/clilogging/common.go | 6 +-- peer/common/common.go | 4 +- 5 files changed, 43 insertions(+), 150 deletions(-) delete mode 100644 core/errors/errors_json.go diff --git a/core/errors/errors.go b/core/errors/errors.go index 1a8f19125a7..341340e462e 100644 --- a/core/errors/errors.go +++ b/core/errors/errors.go @@ -18,7 +18,6 @@ package errors import ( "bytes" - "encoding/json" "fmt" "runtime" @@ -46,24 +45,6 @@ type CallStackError interface { GetComponentCode() ComponentCode GetReasonCode() ReasonCode Message() string - MessageIn(string) string -} - -type errormap map[string]map[string]map[string]string - -var emap errormap - -const language string = "en" - -func init() { - initErrors() -} - -func initErrors() { - e := json.Unmarshal([]byte(errorMapping), &emap) - if e != nil { - panic(e) - } } type callstack []uintptr @@ -77,6 +58,7 @@ type hlError struct { stack callstack componentcode ComponentCode reasoncode ReasonCode + message string args []interface{} stackGetter func(callstack) string } @@ -90,8 +72,9 @@ func newHLError(debug bool) *hlError { } func setupHLError(e *hlError, debug bool) { - e.componentcode = Utility - e.reasoncode = UtilityUnknownError + e.componentcode = "Utility" + e.reasoncode = "UnknownError" + e.message = "An unknown error has occurred." if !debug { e.stackGetter = noopGetStack return @@ -113,12 +96,12 @@ func (h *hlError) GetStack() string { return h.stackGetter(h.stack) } -// GetComponentCode returns the Return code +// GetComponentCode returns the component name func (h *hlError) GetComponentCode() ComponentCode { return h.componentcode } -// GetReasonCode returns the Reason code +// GetReasonCode returns the reason code - i.e. why the error occurred func (h *hlError) GetReasonCode() ReasonCode { return h.reasoncode } @@ -130,43 +113,45 @@ func (h *hlError) GetErrorCode() string { // Message returns the corresponding error message for this error in default // language. -// TODO - figure out the best way to read in system language instead of using -// hard-coded default language func (h *hlError) Message() string { // initialize logging level for errors from core.yaml. it can also be set // for code running on the peer dynamically via CLI using // "peer logging setlevel error " errorLogLevelString, _ := flogging.GetModuleLevel("error") + + message := fmt.Sprintf(h.message, h.args...) if errorLogLevelString == logging.DEBUG.String() { - messageWithCallStack := fmt.Sprintf(emap[fmt.Sprintf("%s", h.componentcode)][fmt.Sprintf("%s", h.reasoncode)][language], h.args...) + "\n" + h.GetStack() - return messageWithCallStack + message = appendCallStack(message, h.GetStack()) } - return fmt.Sprintf(emap[fmt.Sprintf("%s", h.componentcode)][fmt.Sprintf("%s", h.reasoncode)][language], h.args...) + + return message } -// MessageIn returns the corresponding error message for this error in 'language' -func (h *hlError) MessageIn(language string) string { - return fmt.Sprintf(emap[fmt.Sprintf("%s", h.componentcode)][fmt.Sprintf("%s", h.reasoncode)][language], h.args...) +func appendCallStack(message string, callstack string) string { + messageWithCallStack := message + "\n" + callstack + + return messageWithCallStack } // Error creates a CallStackError using a specific Component Code and // Reason Code (no callstack is recorded) -func Error(componentcode ComponentCode, reasoncode ReasonCode, args ...interface{}) CallStackError { - return newCustomError(componentcode, reasoncode, false, args...) +func Error(componentcode ComponentCode, reasoncode ReasonCode, message string, args ...interface{}) CallStackError { + return newCustomError(componentcode, reasoncode, message, false, args...) } // ErrorWithCallstack creates a CallStackError using a specific Component Code and // Reason Code and fills its callstack -func ErrorWithCallstack(componentcode ComponentCode, reasoncode ReasonCode, args ...interface{}) CallStackError { - return newCustomError(componentcode, reasoncode, true, args...) +func ErrorWithCallstack(componentcode ComponentCode, reasoncode ReasonCode, message string, args ...interface{}) CallStackError { + return newCustomError(componentcode, reasoncode, message, true, args...) } -func newCustomError(componentcode ComponentCode, reasoncode ReasonCode, generateStack bool, args ...interface{}) CallStackError { +func newCustomError(componentcode ComponentCode, reasoncode ReasonCode, message string, generateStack bool, args ...interface{}) CallStackError { e := &hlError{} setupHLError(e, generateStack) e.componentcode = componentcode e.reasoncode = reasoncode e.args = args + e.message = message return e } diff --git a/core/errors/errors_json.go b/core/errors/errors_json.go deleted file mode 100644 index 9decafba349..00000000000 --- a/core/errors/errors_json.go +++ /dev/null @@ -1,86 +0,0 @@ -/* - Copyright Digital Asset Holdings, LLC 2016 All Rights Reserved. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package errors - -// Component codes - should be set to the same key in the errorCodes json below -const ( - Utility ComponentCode = "Utility" - Logging ComponentCode = "Logging" - Peer ComponentCode = "Peer" -) - -// Reason codes - should be set to the same key in the errorCodes json below -const ( - // Utility errors - placeholders - UtilityUnknownError ReasonCode = "UtilityUnknownError" - UtilityErrorWithArg ReasonCode = "UtilityErrorWithArg" - - // Logging errors - LoggingUnknownError ReasonCode = "LoggingUnknownError" - LoggingErrorWithArg ReasonCode = "LoggingErrorWithArg" - LoggingNoParameters ReasonCode = "LoggingNoParameters" - LoggingNoLogLevelParameter ReasonCode = "LoggingNoLogLevelParameter" - LoggingInvalidLogLevel ReasonCode = "LoggingInvalidLogLevel" - - // Peer errors - PeerConnectionError ReasonCode = "PeerConnectionError" -) - -// To use this file to define a new component code or reason code, please follow -// these steps: -// 1. Add the new component code below the last component code or add the -// new reason code below the last reason code for the applicable component -// in the json formatted string below -// 2. Define the message in English using "en" as the key -// 3. Add a constant above for each new component/reason code making sure it -// matches the key used in the json formatted string below -// 4. Import "github.com/hyperledger/fabric/core/errors" wherever the error -// will be created. -// 5. Reference the component and reason codes in the call to -// errors.ErrorWithCallstack as follows: -// err = errors.ErrorWithCallstack(errors.Logging, errors.LoggingNoParameters) -// 6. Any code that receives this error will automatically have the callstack -// appended if CORE_LOGGING_ERROR is set to DEBUG in peer/core.yaml, at the -// command line when the peer is started, or if the error module is set -// dynamically using the CLI call "peer logging setlevel error debug" -// 6. For an example in context, see "peer/clilogging/common.go", which creates -// a number of callstack errors, and "peer/clilogging/setlevel.go", which -// receives the errors -const errorMapping string = ` -{"Utility" : - {"UtilityUnknownError" : - {"en": "An unknown error occurred."}, - "UtilityErrorWithArg": - {"en": "An error occurred: %s"} - }, - "Logging": - {"LoggingUnknownError" : - {"en": "A logging error occurred."}, - "LoggingErrorWithArg": - {"en": "A logging error occurred: %s"}, - "LoggingNoParameters": - {"en": "No parameters provided."}, - "LoggingNoLogLevelParameter": - {"en": "No log level provided."}, - "LoggingInvalidLogLevel": - {"en": "Invalid log level provided - %s"} - }, - "Peer" : - {"PeerConnectionError" : - {"en": "Error trying to connect to local peer: %s"} - } -}` diff --git a/core/errors/errors_test.go b/core/errors/errors_test.go index f637c2894e2..d2b76caf0b5 100644 --- a/core/errors/errors_test.go +++ b/core/errors/errors_test.go @@ -25,7 +25,7 @@ import ( ) func TestError(t *testing.T) { - e := Error(Utility, UtilityUnknownError) + e := Error("Utility", "ErrorWithArg", "An unknown error occurred.") s := e.GetStack() if s != "" { t.Fatalf("No error stack should have been recorded.") @@ -34,7 +34,7 @@ func TestError(t *testing.T) { // TestErrorWithArg tests creating an error with a message argument func TestErrorWithArg(t *testing.T) { - e := Error(Utility, UtilityErrorWithArg, "arg1") + e := Error("Utility", "ErrorWithArg", "An error occurred: %s", "arg1") s := e.GetStack() if s != "" { t.Fatalf("No error stack should have been recorded.") @@ -42,7 +42,7 @@ func TestErrorWithArg(t *testing.T) { } func TestErrorWithCallstack(t *testing.T) { - e := ErrorWithCallstack(Utility, UtilityUnknownError) + e := ErrorWithCallstack("Utility", "UnknownError", "An unknown error occurred.") s := e.GetStack() if s == "" { t.Fatalf("No error stack was recorded.") @@ -52,7 +52,7 @@ func TestErrorWithCallstack(t *testing.T) { // TestErrorWithCallstackAndArg tests creating an error with a callstack and // message argument func TestErrorWithCallstackAndArg(t *testing.T) { - e := ErrorWithCallstack(Utility, UtilityErrorWithArg, "arg1") + e := ErrorWithCallstack("Utility", "ErrorWithArg", "An error occurred: %s", "arg1") s := e.GetStack() if s == "" { t.Fatalf("No error stack was recorded.") @@ -67,7 +67,7 @@ func TestErrorWithCallstackMessage(t *testing.T) { // to the error message flogging.SetModuleLevel("error", "debug") - e := ErrorWithCallstack(Utility, UtilityUnknownError) + e := ErrorWithCallstack("Utility", "ErrorWithArg", "An unknown error occurred.") s := e.GetStack() if s == "" { t.Fatalf("No error stack was recorded.") @@ -85,7 +85,7 @@ func ExampleError() { // not be appended to the error message flogging.SetModuleLevel("error", "warning") - err := ErrorWithCallstack(Utility, UtilityUnknownError) + err := ErrorWithCallstack("Utility", "UnknownError", "An unknown error occurred.") if err != nil { fmt.Printf("%s\n", err.Error()) @@ -93,25 +93,23 @@ func ExampleError() { fmt.Printf("%s\n", err.GetComponentCode()) fmt.Printf("%s\n", err.GetReasonCode()) fmt.Printf("%s\n", err.Message()) - fmt.Printf("%s\n", err.MessageIn("en")) // Output: // An unknown error occurred. - // Utility-UtilityUnknownError + // Utility-UnknownError // Utility - // UtilityUnknownError - // An unknown error occurred. + // UnknownError // An unknown error occurred. } } // ExampleErrorWithArg tests the output for a sample error with a message // argument -func ExampleUtilityErrorWithArg() { +func Example_utilityErrorWithArg() { // when the 'error' module is set to anything but debug, the callstack will // not be appended to the error message flogging.SetModuleLevel("error", "warning") - err := ErrorWithCallstack(Utility, UtilityErrorWithArg, "arg1") + err := ErrorWithCallstack("Utility", "ErrorWithArg", "An error occurred: %s", "arg1") if err != nil { fmt.Printf("%s\n", err.Error()) @@ -119,25 +117,23 @@ func ExampleUtilityErrorWithArg() { fmt.Printf("%s\n", err.GetComponentCode()) fmt.Printf("%s\n", err.GetReasonCode()) fmt.Printf("%s\n", err.Message()) - fmt.Printf("%s\n", err.MessageIn("en")) // Output: // An error occurred: arg1 - // Utility-UtilityErrorWithArg + // Utility-ErrorWithArg // Utility - // UtilityErrorWithArg - // An error occurred: arg1 + // ErrorWithArg // An error occurred: arg1 } } -// ExampleLoggingInvalidLogLevel tests the output for a logging error where +// Example_loggingInvalidLevel tests the output for a logging error where // and an invalid log level has been provided -func ExampleLoggingInvalidLogLevel() { +func Example_loggingInvalidLevel() { // when the 'error' module is set to anything but debug, the callstack will // not be appended to the error message flogging.SetModuleLevel("error", "warning") - err := ErrorWithCallstack(Logging, LoggingInvalidLogLevel, "invalid") + err := ErrorWithCallstack("Logging", "InvalidLevel", "Invalid log level provided - %s", "invalid") if err != nil { fmt.Printf("%s\n", err.Error()) @@ -145,13 +141,11 @@ func ExampleLoggingInvalidLogLevel() { fmt.Printf("%s\n", err.GetComponentCode()) fmt.Printf("%s\n", err.GetReasonCode()) fmt.Printf("%s\n", err.Message()) - fmt.Printf("%s\n", err.MessageIn("en")) // Output: // Invalid log level provided - invalid - // Logging-LoggingInvalidLogLevel + // Logging-InvalidLevel // Logging - // LoggingInvalidLogLevel - // Invalid log level provided - invalid + // InvalidLevel // Invalid log level provided - invalid } } diff --git a/peer/clilogging/common.go b/peer/clilogging/common.go index 80dc3a6aa41..fc298dab051 100644 --- a/peer/clilogging/common.go +++ b/peer/clilogging/common.go @@ -28,19 +28,19 @@ func checkLoggingCmdParams(cmd *cobra.Command, args []string) error { // check that at least one parameter is passed in if len(args) == 0 { - err = errors.ErrorWithCallstack(errors.Logging, errors.LoggingNoParameters) + err = errors.ErrorWithCallstack("Logging", "NoParameters", "No parameters provided.") return err } if cmd.Name() == "setlevel" { // check that log level parameter is provided if len(args) == 1 { - err = errors.ErrorWithCallstack(errors.Logging, errors.LoggingNoLogLevelParameter) + err = errors.ErrorWithCallstack("Logging", "NoLevelParameter", "No log level provided.") } else { // check that log level is valid. if not, err is set _, err = logging.LogLevel(args[1]) if err != nil { - err = errors.ErrorWithCallstack(errors.Logging, errors.LoggingInvalidLogLevel, args[1]) + err = errors.ErrorWithCallstack("Logging", "InvalidLevel", "Invalid log level provided - %s", args[1]) } } } diff --git a/peer/common/common.go b/peer/common/common.go index fed47c0a402..b04e6743303 100644 --- a/peer/common/common.go +++ b/peer/common/common.go @@ -79,7 +79,7 @@ func InitCrypto(mspMgrConfigDir string) error { func GetEndorserClient() (pb.EndorserClient, error) { clientConn, err := peer.NewPeerClientConnection() if err != nil { - err = errors.ErrorWithCallstack(errors.Peer, errors.PeerConnectionError, err.Error()) + err = errors.ErrorWithCallstack("Peer", "ConnectionError", "Error trying to connect to local peer: %s", err.Error()) return nil, err } endorserClient := pb.NewEndorserClient(clientConn) @@ -90,7 +90,7 @@ func GetEndorserClient() (pb.EndorserClient, error) { func GetAdminClient() (pb.AdminClient, error) { clientConn, err := peer.NewPeerClientConnection() if err != nil { - err = errors.ErrorWithCallstack(errors.Peer, errors.PeerConnectionError, err.Error()) + err = errors.ErrorWithCallstack("Peer", "ConnectionError", "Error trying to connect to local peer: %s", err.Error()) return nil, err } adminClient := pb.NewAdminClient(clientConn)