diff --git a/cmd/main.go b/cmd/main.go index b69a2cc2..7ac3a74f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -19,6 +19,7 @@ func main() { var extensionRegistryURL = flag.String("extensionRegistryURL", "", "URL of the extension registry index used to find available extensions or the path of a local directory") var serverAddress = flag.String("serverAddress", ":8080", `Server address, e.g. ":8080" (all network interfaces) or "localhost:8080" (only local interface)`) var openAPIOutputPath = flag.String("openAPIOutputPath", "", "Generate the OpenAPI spec at the given path instead of starting the server") + var addCauseToInternalServerError = flag.Bool("addCauseToInternalServerError", false, "Add cause of internal server errors (status 500) to the error message. Don't use this in production!") flag.Parse() log.SetLevel(log.DebugLevel) log.SetFormatter(&simpleFormatter{}) @@ -28,17 +29,17 @@ func main() { panic(fmt.Sprintf("failed to generate OpenAPI to %q: %v", *openAPIOutputPath, err)) } } else { - startServer(*extensionRegistryURL, *serverAddress) + startServer(*extensionRegistryURL, *serverAddress, *addCauseToInternalServerError) } } -func startServer(pathToExtensionFolder string, serverAddress string) { +func startServer(pathToExtensionFolder string, serverAddress string, addCauseToInternalServerError bool) { log.Printf("Starting extension manager with extension folder %q", pathToExtensionFolder) controller := extensionController.CreateWithConfig(extensionController.ExtensionManagerConfig{ ExtensionRegistryURL: pathToExtensionFolder, ExtensionSchema: restAPI.EXTENSION_SCHEMA_NAME, BucketFSBasePath: "/buckets/bfsdefault/default/"}) - restApi := restAPI.Create(controller, serverAddress) + restApi := restAPI.Create(controller, serverAddress, addCauseToInternalServerError) restApi.Serve() } diff --git a/dependencies.md b/dependencies.md index b29d5ff7..50b8ac7b 100644 --- a/dependencies.md +++ b/dependencies.md @@ -99,7 +99,7 @@ | [mockito-junit-jupiter][75] | [The MIT License][76] | | [udf-debugging-java][77] | [MIT License][78] | | [Maven Project Version Getter][79] | [MIT License][80] | -| [SLF4J JDK14 Binding][81] | [MIT License][22] | +| [SLF4J JDK14 Provider][81] | [MIT License][22] | ### Plugin Dependencies diff --git a/doc/changes/changes_0.5.1.md b/doc/changes/changes_0.5.1.md index a2cffaaf..7f6ad2b8 100644 --- a/doc/changes/changes_0.5.1.md +++ b/doc/changes/changes_0.5.1.md @@ -1,4 +1,4 @@ -# Extension Manager 0.5.1, released 2023-??-?? +# Extension Manager 0.5.1, released 2023-09-08 Code name: Support `select` parameter type @@ -7,15 +7,17 @@ Code name: Support `select` parameter type This release contains the following changes: * It adds support for the `select` parameter type. * In Integration test class `PreviousExtensionVersion` the builder property `adapterFileName` is now optional. This is useful for extensions that don't need an adapter file like Lua based virtual schemas. +* Command line flag `-addCauseToInternalServerError` of the standalone server now allows adding the root cause error message to internal server errors (status 500). + + This is helpful during debugging because the error message contains the actual root cause and you don't need to check the log for errors. The Java integration test framework `extension-manager-integration-test-java` enables this flag automatically. + + ⚠️Warning⚠️: Do not enable this flag in production environments as this might leak internal information. ## Features * #132: Added support for the `select` parameter type * #134: Made adapter file optional for `PreviousExtensionVersion` - -## Documentation - -* #129: Improved description of deployment process +* #131: Added command line flag `-addCauseToInternalServerError` ## Dependency Updates @@ -41,6 +43,7 @@ This release contains the following changes: #### Test Dependency Updates * Updated `org.mockito:mockito-junit-jupiter:5.4.0` to `5.5.0` +* Updated `org.slf4j:slf4j-jdk14:2.0.7` to `2.0.9` #### Plugin Dependency Updates diff --git a/extension-manager-integration-test-java/pom.xml b/extension-manager-integration-test-java/pom.xml index e7781400..965bc9b2 100644 --- a/extension-manager-integration-test-java/pom.xml +++ b/extension-manager-integration-test-java/pom.xml @@ -73,7 +73,7 @@ org.slf4j slf4j-jdk14 - 2.0.7 + 2.0.9 test diff --git a/extension-manager-integration-test-java/src/main/java/com/exasol/extensionmanager/itest/ExtensionManagerProcess.java b/extension-manager-integration-test-java/src/main/java/com/exasol/extensionmanager/itest/ExtensionManagerProcess.java index 7398249e..c5e7d712 100644 --- a/extension-manager-integration-test-java/src/main/java/com/exasol/extensionmanager/itest/ExtensionManagerProcess.java +++ b/extension-manager-integration-test-java/src/main/java/com/exasol/extensionmanager/itest/ExtensionManagerProcess.java @@ -4,6 +4,7 @@ import java.net.ServerSocket; import java.nio.file.Path; import java.time.Duration; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -32,8 +33,9 @@ static ExtensionManagerProcess start(final Path extensionManagerBinary, final Pa final int port = findOpenPort(); LOGGER.info(() -> "Starting extension manager " + extensionManagerBinary + " on port " + port + " with extension folder " + extensionFolder + "..."); - final List command = List.of(extensionManagerBinary.toString(), "-extensionRegistryURL", - extensionFolder.toString(), "-serverAddress", "localhost:" + port); + final List command = new ArrayList<>(List.of(extensionManagerBinary.toString(), "-extensionRegistryURL", + extensionFolder.toString(), "-serverAddress", "localhost:" + port)); + addFlagIfSupported(extensionManagerBinary, "-addCauseToInternalServerError", command); final ServerStartupConsumer serverPortConsumer = new ServerStartupConsumer(); final SimpleProcess process = SimpleProcess.start(command, @@ -49,6 +51,22 @@ static ExtensionManagerProcess start(final Path extensionManagerBinary, final Pa return new ExtensionManagerProcess(process, port); } + private static void addFlagIfSupported(final Path extensionManagerBinary, final String flag, + final List command) { + if (supportsFlag(extensionManagerBinary, flag)) { + command.add(flag); + } + } + + static boolean supportsFlag(final Path extensionManagerBinary, final String flag) { + final String helpContent = getHelpContent(extensionManagerBinary); + return helpContent.contains(flag); + } + + static String getHelpContent(final Path extensionManagerBinary) { + return SimpleProcess.start(List.of(extensionManagerBinary.toString(), "-help"), Duration.ofSeconds(3)); + } + private static int findOpenPort() { try (ServerSocket socket = new ServerSocket(0)) { return socket.getLocalPort(); diff --git a/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/PreviousVersionManagerTest.java b/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/PreviousVersionManagerTest.java index bf3bb6bb..f60f99a4 100644 --- a/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/PreviousVersionManagerTest.java +++ b/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/PreviousVersionManagerTest.java @@ -30,7 +30,7 @@ @ExtendWith(MockitoExtension.class) class PreviousVersionManagerTest { - + private static final String BASE_URL = "https://extensions-internal.exasol.com"; @Mock private ExtensionManagerSetup setupMock; @Mock @@ -56,8 +56,8 @@ void newVersion() { @Test void fetchExtension() throws IOException { - final String extensionId = testee.fetchExtension(URI.create( - "https://extensions-internal.exasol.com/com.exasol/s3-document-files-virtual-schema/2.6.2/s3-vs-extension.js")); + final String extensionId = testee.fetchExtension( + URI.create(BASE_URL + "/com.exasol/s3-document-files-virtual-schema/2.6.2/s3-vs-extension.js")); final Path file = tempDir.resolve(extensionId); assertAll(() -> assertTrue(Files.exists(file), "file downloaded"), () -> assertThat(Files.size(file), equalTo(20875L))); @@ -66,7 +66,7 @@ void fetchExtension() throws IOException { @Test void fetchExtensionFailsForMissingFile() throws IOException { - final URI uri = URI.create("https://extensions-internal.exasol.com/no-such-file"); + final URI uri = URI.create(BASE_URL + "/no-such-file"); final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> testee.fetchExtension(uri)); assertThat(exception.getMessage(), @@ -85,8 +85,8 @@ void fetchExtensionFails() throws IOException { @Test void prepareBucketFsFile() throws FileNotFoundException, BucketAccessException, TimeoutException { - testee.prepareBucketFsFile(URI.create( - "https://extensions-internal.exasol.com/com.exasol/s3-document-files-virtual-schema/2.6.2/s3-vs-extension.js"), + testee.prepareBucketFsFile( + URI.create(BASE_URL + "/com.exasol/s3-document-files-virtual-schema/2.6.2/s3-vs-extension.js"), "filename"); verify(bucketMock).uploadFile(any(), eq("filename")); } diff --git a/pkg/restAPI/apiContext.go b/pkg/restAPI/apiContext.go index 76842ae3..1fd6b63d 100644 --- a/pkg/restAPI/apiContext.go +++ b/pkg/restAPI/apiContext.go @@ -4,10 +4,14 @@ import ( "github.com/exasol/extension-manager/pkg/extensionController" ) -func NewApiContext(controller extensionController.TransactionController) *ApiContext { - return &ApiContext{Controller: controller} +func NewApiContext(controller extensionController.TransactionController, addCauseToInternalServerError bool) *ApiContext { + return &ApiContext{ + Controller: controller, + addCauseToInternalServerError: addCauseToInternalServerError, + } } type ApiContext struct { - Controller extensionController.TransactionController + Controller extensionController.TransactionController + addCauseToInternalServerError bool } diff --git a/pkg/restAPI/dbConnection.go b/pkg/restAPI/dbConnection.go index d1c036de..9e38811c 100644 --- a/pkg/restAPI/dbConnection.go +++ b/pkg/restAPI/dbConnection.go @@ -15,17 +15,20 @@ import ( ) type generalHandlerFunc = func(writer http.ResponseWriter, request *http.Request) -type dbHandler = func(db *sql.DB, writer http.ResponseWriter, request *http.Request) +type dbHandler = func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error -func adaptDbHandler(f dbHandler) generalHandlerFunc { +func adaptDbHandler(apiContext *ApiContext, handler dbHandler) generalHandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { db, err := openDBRequest(request) if err != nil { - HandleError(request.Context(), writer, err) + handleError(request.Context(), apiContext, writer, err) return } defer closeDBRequest(db) - f(db, writer, request) + err = handler(db, writer, request) + if err != nil { + handleError(request.Context(), apiContext, writer, err) + } } } diff --git a/pkg/restAPI/public.go b/pkg/restAPI/public.go index 83b44ce2..31f5ccff 100644 --- a/pkg/restAPI/public.go +++ b/pkg/restAPI/public.go @@ -22,17 +22,17 @@ const ( /* [impl -> dsn~go-library~1]. */ func AddPublicEndpoints(api *openapi.API, config extensionController.ExtensionManagerConfig) error { controller := extensionController.CreateWithConfig(config) - return addPublicEndpointsWithController(api, controller) + return addPublicEndpointsWithController(api, false, controller) } /* [impl -> dsn~rest-interface~1] */ /* [impl -> dsn~openapi-spec~1]. */ -func addPublicEndpointsWithController(api *openapi.API, controller extensionController.TransactionController) error { +func addPublicEndpointsWithController(api *openapi.API, addCauseToInternalServerError bool, controller extensionController.TransactionController) error { api.AddTag(TagExtension, "List and install extensions") api.AddTag(TagInstallation, "List and uninstall installed extensions") api.AddTag(TagInstance, "Calls to list, create and remove instances of an extension") - apiContext := NewApiContext(controller) + apiContext := NewApiContext(controller, addCauseToInternalServerError) if err := api.Get(ListAvailableExtensions(apiContext)); err != nil { return err diff --git a/pkg/restAPI/requestCreateInstance.go b/pkg/restAPI/requestCreateInstance.go index 43bbaa44..a532f9a5 100644 --- a/pkg/restAPI/requestCreateInstance.go +++ b/pkg/restAPI/requestCreateInstance.go @@ -33,17 +33,16 @@ func CreateInstance(apiContext *ApiContext) *openapi.Post { AddParameter("extensionId", openapi.STRING, "ID of the installed extension for which to create an instance"). AddParameter("extensionVersion", openapi.STRING, "Version of the installed extension for which to create an instance"). Add("instances"), - HandlerFunc: adaptDbHandler(handleCreateInstance(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleCreateInstance(apiContext)), } } func handleCreateInstance(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { requestBody := CreateInstanceRequest{} err := DecodeJSONBody(writer, request, &requestBody) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } var parameters []extensionController.ParameterValue for _, p := range requestBody.ParameterValues { @@ -53,11 +52,10 @@ func handleCreateInstance(apiContext *ApiContext) dbHandler { extensionVersion := chi.URLParam(request, "extensionVersion") instance, err := apiContext.Controller.CreateInstance(request.Context(), db, extensionId, extensionVersion, parameters) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } logrus.Debugf("Created instance %q", instance) - SendJSON(request.Context(), writer, CreateInstanceResponse{InstanceId: instance.Id, InstanceName: instance.Name}) + return SendJSON(request.Context(), writer, CreateInstanceResponse{InstanceId: instance.Id, InstanceName: instance.Name}) } } diff --git a/pkg/restAPI/requestDeleteInstance.go b/pkg/restAPI/requestDeleteInstance.go index 2ee0d75e..c00f5ae1 100644 --- a/pkg/restAPI/requestDeleteInstance.go +++ b/pkg/restAPI/requestDeleteInstance.go @@ -27,20 +27,19 @@ func DeleteInstance(apiContext *ApiContext) *openapi.Delete { AddParameter("extensionVersion", openapi.STRING, "The version of the installed extension for which to delete an instance"). Add("instances"). AddParameter("instanceId", openapi.STRING, "The ID of the instance to delete"), - HandlerFunc: adaptDbHandler(handleDeleteInstance(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleDeleteInstance(apiContext)), } } func handleDeleteInstance(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { extensionId := chi.URLParam(request, "extensionId") extensionVersion := chi.URLParam(request, "extensionVersion") instanceId := chi.URLParam(request, "instanceId") err := apiContext.Controller.DeleteInstance(request.Context(), db, extensionId, extensionVersion, instanceId) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } - SendNoContent(request.Context(), writer) + return SendNoContent(request.Context(), writer) } } diff --git a/pkg/restAPI/requestGetExtensionDetails.go b/pkg/restAPI/requestGetExtensionDetails.go index f9f87cad..1a2529bc 100644 --- a/pkg/restAPI/requestGetExtensionDetails.go +++ b/pkg/restAPI/requestGetExtensionDetails.go @@ -32,21 +32,20 @@ func GetExtensionDetails(apiContext *ApiContext) *openapi.Get { Add("extensions"). AddParameter("extensionId", openapi.STRING, "ID of the extension"). AddParameter("extensionVersion", openapi.STRING, "Version of the extension"), - HandlerFunc: adaptDbHandler(handleGetParameterDefinitions(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleGetParameterDefinitions(apiContext)), } } func handleGetParameterDefinitions(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { extensionId := chi.URLParam(request, "extensionId") extensionVersion := chi.URLParam(request, "extensionVersion") definitions, err := apiContext.Controller.GetParameterDefinitions(request.Context(), db, extensionId, extensionVersion) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } response := ExtensionDetailsResponse{Id: extensionId, Version: extensionVersion, ParamDefinitions: convertParamDefinitions(definitions)} - SendJSON(request.Context(), writer, response) + return SendJSON(request.Context(), writer, response) } } diff --git a/pkg/restAPI/requestInstallExtension.go b/pkg/restAPI/requestInstallExtension.go index 2c6b2c59..6a37e358 100644 --- a/pkg/restAPI/requestInstallExtension.go +++ b/pkg/restAPI/requestInstallExtension.go @@ -28,21 +28,19 @@ func InstallExtension(apiContext *ApiContext) *openapi.Put { AddParameter("extensionId", openapi.STRING, "ID of the extension to install"). AddParameter("extensionVersion", openapi.STRING, "Version of the extension to install"). Add("install"), - HandlerFunc: adaptDbHandler(handleInstallExtension(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleInstallExtension(apiContext)), } } func handleInstallExtension(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { extensionId := chi.URLParam(request, "extensionId") extensionVersion := chi.URLParam(request, "extensionVersion") err := apiContext.Controller.InstallExtension(request.Context(), db, extensionId, extensionVersion) - if err != nil { - HandleError(request.Context(), writer, err) - return + return err } - SendNoContent(request.Context(), writer) + return SendNoContent(request.Context(), writer) } } diff --git a/pkg/restAPI/requestListAvailableExtensions.go b/pkg/restAPI/requestListAvailableExtensions.go index bc6e37ea..7b82937a 100644 --- a/pkg/restAPI/requestListAvailableExtensions.go +++ b/pkg/restAPI/requestListAvailableExtensions.go @@ -31,20 +31,19 @@ func ListAvailableExtensions(apiContext *ApiContext) *openapi.Get { }}, }, Path: newPathWithDbQueryParams().Add("extensions"), - HandlerFunc: adaptDbHandler(handleListAvailableExtensions(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleListAvailableExtensions(apiContext)), } } func handleListAvailableExtensions(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { extensions, err := apiContext.Controller.GetAllExtensions(request.Context(), db) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } response := convertResponse(extensions) log.Debugf("Got %d available extensions", len(response.Extensions)) - SendJSON(request.Context(), writer, response) + return SendJSON(request.Context(), writer, response) } } diff --git a/pkg/restAPI/requestListInstalledExtensions.go b/pkg/restAPI/requestListInstalledExtensions.go index b3c80f68..8f58875a 100644 --- a/pkg/restAPI/requestListInstalledExtensions.go +++ b/pkg/restAPI/requestListInstalledExtensions.go @@ -23,19 +23,18 @@ func ListInstalledExtensions(apiContext *ApiContext) *openapi.Get { }}, }, Path: newPathWithDbQueryParams().Add("installations"), - HandlerFunc: adaptDbHandler(handleListInstalledExtensions(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleListInstalledExtensions(apiContext)), } } func handleListInstalledExtensions(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { installations, err := apiContext.Controller.GetInstalledExtensions(request.Context(), db) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } response := createResponse(installations) - SendJSON(request.Context(), writer, response) + return SendJSON(request.Context(), writer, response) } } diff --git a/pkg/restAPI/requestListInstances.go b/pkg/restAPI/requestListInstances.go index 31252d6c..8b505f41 100644 --- a/pkg/restAPI/requestListInstances.go +++ b/pkg/restAPI/requestListInstances.go @@ -26,24 +26,23 @@ func ListInstances(apiContext *ApiContext) *openapi.Get { AddParameter("extensionId", openapi.STRING, "The ID of the installed extension for which to get the instances"). AddParameter("extensionVersion", openapi.STRING, "The version of the installed extension for which to get the instances"). Add("instances"), - HandlerFunc: adaptDbHandler(handleListInstances(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleListInstances(apiContext)), } } func handleListInstances(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { id := chi.URLParam(request, "extensionId") version := chi.URLParam(request, "extensionVersion") rawInstances, err := apiContext.Controller.FindInstances(request.Context(), db, id, version) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } instances := make([]Instance, 0, len(rawInstances)) for _, i := range rawInstances { instances = append(instances, Instance{Id: i.Id, Name: i.Name}) } - SendJSON(request.Context(), writer, ListInstancesResponse{Instances: instances}) + return SendJSON(request.Context(), writer, ListInstancesResponse{Instances: instances}) } } diff --git a/pkg/restAPI/requestResponseHelper.go b/pkg/restAPI/requestResponseHelper.go index f1c94f42..05a250a3 100644 --- a/pkg/restAPI/requestResponseHelper.go +++ b/pkg/restAPI/requestResponseHelper.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "net/http" "strings" @@ -20,15 +21,15 @@ const ( ) // SendJSON converts the given data to JSON and sends it to the writer. -func SendJSON(ctx context.Context, writer http.ResponseWriter, data interface{}) { - SendJSONWithStatus(ctx, 200, writer, data) +func SendJSON(ctx context.Context, writer http.ResponseWriter, data interface{}) error { + return SendJSONWithStatus(ctx, 200, writer, data) } -func SendNoContent(ctx context.Context, writer http.ResponseWriter) { - SendJSONWithStatus(ctx, http.StatusNoContent, writer, nil) +func SendNoContent(ctx context.Context, writer http.ResponseWriter) error { + return SendJSONWithStatus(ctx, http.StatusNoContent, writer, nil) } -func SendJSONWithStatus(ctx context.Context, status int, writer http.ResponseWriter, data interface{}) { +func SendJSONWithStatus(ctx context.Context, status int, writer http.ResponseWriter, data interface{}) error { logger := GetLogger(ctx) writer.Header().Set(HeaderContentType, ContentTypeJson) writer.WriteHeader(status) @@ -46,20 +47,24 @@ func SendJSONWithStatus(ctx context.Context, status int, writer http.ResponseWri encoder.SetEscapeHTML(false) encodeErr := encoder.Encode(data) if encodeErr != nil { - logger.Warnf("Could not send json: %s", encodeErr.Error()) + err := fmt.Errorf("Could not send json: %w", encodeErr) + logger.Warnf(err.Error()) + return err + } } else if status != http.StatusNoContent { logger.Warnf("No response data for status %d", status) } + return nil } -func HandleError(context context.Context, writer http.ResponseWriter, err error) { +func handleError(context context.Context, apiContext *ApiContext, writer http.ResponseWriter, err error) { log.Errorf("Error processing request: %v", err) errorToSend := apiErrors.UnwrapAPIError(err) - sendError(errorToSend, context, writer) + sendError(errorToSend, context, apiContext, writer) } -func sendError(a *apiErrors.APIError, context context.Context, writer http.ResponseWriter) { +func sendError(a *apiErrors.APIError, context context.Context, apiContext *ApiContext, writer http.ResponseWriter) { writer.Header().Set(HeaderContentType, ContentTypeJson) writer.WriteHeader(a.Status) if context != nil && a.Status != http.StatusUnauthorized { @@ -69,7 +74,9 @@ func sendError(a *apiErrors.APIError, context context.Context, writer http.Respo } a.APIID = getContextValue(context, APIIDKey) } - + if apiContext.addCauseToInternalServerError && a.Status == http.StatusInternalServerError && a.OriginalError != nil { + a.Message = a.Message + ": " + a.OriginalError.Error() + } err := json.NewEncoder(writer).Encode(a) if err != nil { logger := GetLogger(context) diff --git a/pkg/restAPI/requestTestUtil.go b/pkg/restAPI/requestTestUtil.go index 00b14100..ec35bf1a 100644 --- a/pkg/restAPI/requestTestUtil.go +++ b/pkg/restAPI/requestTestUtil.go @@ -12,11 +12,11 @@ import ( "github.com/stretchr/testify/suite" ) -func startRestApi(suite *suite.Suite, controller extensionController.TransactionController) *baseRestAPITest { +func startRestApi(suite *suite.Suite, addCauseToInternalServerError bool, controller extensionController.TransactionController) *baseRestAPITest { hostAndPort := "localhost:8081" api := baseRestAPITest{ suite: suite, - restAPI: Create(controller, hostAndPort), + restAPI: Create(controller, hostAndPort, addCauseToInternalServerError), baseUrl: fmt.Sprintf("http://%s", hostAndPort)} api.restAPI.StartInBackground() return &api diff --git a/pkg/restAPI/requestUninstallExtension.go b/pkg/restAPI/requestUninstallExtension.go index dfb88ae5..e2583025 100644 --- a/pkg/restAPI/requestUninstallExtension.go +++ b/pkg/restAPI/requestUninstallExtension.go @@ -22,19 +22,18 @@ func UninstallExtension(apiContext *ApiContext) *openapi.Delete { Add("installations"). AddParameter("extensionId", openapi.STRING, "The ID of the installed extension to uninstall"). AddParameter("extensionVersion", openapi.STRING, "The version of the installed extension to uninstall"), - HandlerFunc: adaptDbHandler(handleUninstallExtension(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleUninstallExtension(apiContext)), } } func handleUninstallExtension(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { extensionId := chi.URLParam(request, "extensionId") version := chi.URLParam(request, "extensionVersion") err := apiContext.Controller.UninstallExtension(request.Context(), db, extensionId, version) if err != nil { - HandleError(request.Context(), writer, err) - return + return err } - SendNoContent(request.Context(), writer) + return SendNoContent(request.Context(), writer) } } diff --git a/pkg/restAPI/requestUpgradeExtension.go b/pkg/restAPI/requestUpgradeExtension.go index 85542e5f..c0b4fa60 100644 --- a/pkg/restAPI/requestUpgradeExtension.go +++ b/pkg/restAPI/requestUpgradeExtension.go @@ -33,21 +33,20 @@ func UpgradeExtension(apiContext *ApiContext) *openapi.Post { Add("installations"). AddParameter("extensionId", openapi.STRING, "The ID of the installed extension to upgrade"). Add("upgrade"), - HandlerFunc: adaptDbHandler(handleUpgradeExtension(apiContext)), + HandlerFunc: adaptDbHandler(apiContext, handleUpgradeExtension(apiContext)), } } func handleUpgradeExtension(apiContext *ApiContext) dbHandler { - return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) { + return func(db *sql.DB, writer http.ResponseWriter, request *http.Request) error { extensionId := chi.URLParam(request, "extensionId") result, err := apiContext.Controller.UpgradeExtension(request.Context(), db, extensionId) if err != nil { logrus.Warnf("Upgrading of extension %q failed: %v", extensionId, err) - HandleError(request.Context(), writer, err) - return + return err } logrus.Infof("Successfully upgraded extension %q from version %s to %s", extensionId, result.PreviousVersion, result.NewVersion) - SendJSON(request.Context(), writer, UpgradeExtensionResponse{ + return SendJSON(request.Context(), writer, UpgradeExtensionResponse{ PreviousVersion: result.PreviousVersion, NewVersion: result.NewVersion}) } diff --git a/pkg/restAPI/restApi.go b/pkg/restAPI/restApi.go index dfeaccc8..bcef424b 100644 --- a/pkg/restAPI/restApi.go +++ b/pkg/restAPI/restApi.go @@ -23,16 +23,21 @@ type RestAPI interface { } // Create creates a new RestAPI. -func Create(controller extensionController.TransactionController, serverAddress string) RestAPI { - return &restAPIImpl{controller: controller, serverAddress: serverAddress} +func Create(controller extensionController.TransactionController, serverAddress string, addCauseToInternalServerError bool) RestAPI { + return &restAPIImpl{ + controller: controller, + serverAddress: serverAddress, + addCauseToInternalServerError: addCauseToInternalServerError, + } } type restAPIImpl struct { - controller extensionController.TransactionController - serverAddress string - server *http.Server - stopped *bool - stoppedMutex *sync.Mutex + controller extensionController.TransactionController + addCauseToInternalServerError bool + serverAddress string + server *http.Server + stopped *bool + stoppedMutex *sync.Mutex } func (api *restAPIImpl) StartInBackground() { @@ -46,7 +51,7 @@ func (api *restAPIImpl) Serve() { } api.setStopped(false) - handler, _, err := setupStandaloneAPI(api.controller) + handler, _, err := setupStandaloneAPI(api.controller, api.addCauseToInternalServerError) if err != nil { log.Fatalf("failed to setup api: %v", err) } diff --git a/pkg/restAPI/restApi_i_test.go b/pkg/restAPI/restApi_i_test.go index 7c5143c6..f79c8f60 100644 --- a/pkg/restAPI/restApi_i_test.go +++ b/pkg/restAPI/restApi_i_test.go @@ -44,7 +44,7 @@ func (suite *RestAPIIntegrationTestSuite) TearDownSuite() { func (suite *RestAPIIntegrationTestSuite) SetupTest() { // [itest -> dsn~extension-registry~1] ctrl := extensionController.Create(suite.registryServer.IndexUrl(), EXTENSION_SCHEMA) - suite.restApi = startRestApi(&suite.Suite, ctrl) + suite.restApi = startRestApi(&suite.Suite, false, ctrl) suite.registryServer.Reset() suite.registryServer.SetRegistryContent(`{"extensions":[]}`) } diff --git a/pkg/restAPI/restApi_test.go b/pkg/restAPI/restApi_test.go index 6ae6f08c..3f494ae5 100644 --- a/pkg/restAPI/restApi_test.go +++ b/pkg/restAPI/restApi_test.go @@ -34,6 +34,8 @@ const ( VALID_DB_ARGS = "?dbHost=host&dbPort=8563" ) +var mockError = fmt.Errorf("mock error") + func TestRestApiSuite(t *testing.T) { suite.Run(t, new(RestAPISuite)) } @@ -44,7 +46,7 @@ func (suite *RestAPISuite) SetupSuite() { func (suite *RestAPISuite) SetupTest() { suite.controller = &mockExtensionController{} - suite.restApi = startRestApi(&suite.Suite, suite.controller) + suite.restApi = startRestApi(&suite.Suite, true, suite.controller) } func (suite *RestAPISuite) TearDownTest() { @@ -53,7 +55,7 @@ func (suite *RestAPISuite) TearDownTest() { func (suite *RestAPISuite) TestStopWithoutStartFails() { controller := &mockExtensionController{} - restAPI := Create(controller, "localhost:8082") + restAPI := Create(controller, "localhost:8082", false) suite.Panics(restAPI.Stop) } @@ -74,9 +76,9 @@ func (suite *RestAPISuite) TestGetInstallationsSuccessfully() { } func (suite *RestAPISuite) TestGetInstallationsFailed() { - suite.controller.On("GetInstalledExtensions", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) + suite.controller.On("GetInstalledExtensions", mock.Anything, mock.Anything).Return(nil, mockError) responseString := suite.makeRequest("GET", LIST_INSTALLED_EXTENSIONS+VALID_DB_ARGS, "", 500) - suite.Regexp(`{"code":500,"message":"Internal server error",`, responseString) + suite.isInternalServerError(responseString, mockError) } // GetAllExtensions @@ -95,9 +97,9 @@ func (suite *RestAPISuite) TestGetAllExtensionsSuccessfully() { } func (suite *RestAPISuite) TestGetAllExtensionsFails() { - suite.controller.On("GetAllExtensions", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) + suite.controller.On("GetAllExtensions", mock.Anything, mock.Anything).Return(nil, mockError) responseString := suite.makeRequest("GET", LIST_AVAILABLE_EXTENSIONS+VALID_DB_ARGS, "", 500) - suite.Regexp(`{"code":500,"message":"Internal server error",.*`, responseString) + suite.isInternalServerError(responseString, mockError) } // GetExtensionDetails @@ -116,9 +118,9 @@ func (suite *RestAPISuite) TestGetExtensionDetailsSuccessfully() { } func (suite *RestAPISuite) TestGetExtensionDetailsFails() { - suite.controller.On("GetParameterDefinitions", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(nil, fmt.Errorf("mock error")) + suite.controller.On("GetParameterDefinitions", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(nil, mockError) responseString := suite.makeRequest("GET", GET_EXTENSION_DETAILS+VALID_DB_ARGS, "", 500) - suite.Regexp(`{"code":500,"message":"Internal server error",.*`, responseString) + suite.isInternalServerError(responseString, mockError) } // Install extension @@ -134,9 +136,9 @@ func (suite *RestAPISuite) TestInstallExtensionsSuccessfully() { } func (suite *RestAPISuite) TestInstallExtensionsFailed() { - suite.controller.On("InstallExtension", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(fmt.Errorf("mock error")) + suite.controller.On("InstallExtension", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(mockError) responseString := suite.makeRequest("PUT", INSTALL_EXT_URL+VALID_DB_ARGS, `{}`, 500) - suite.Regexp(`{"code":500,"message":"Internal server error",.*`, responseString) + suite.isInternalServerError(responseString, mockError) } // Uninstall extension @@ -152,9 +154,9 @@ func (suite *RestAPISuite) TestUninstallExtensionsSuccessfully() { } func (suite *RestAPISuite) TestUninstallExtensionsFailed() { - suite.controller.On("UninstallExtension", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(fmt.Errorf("mock error")) + suite.controller.On("UninstallExtension", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(mockError) responseString := suite.makeRequest("DELETE", UNINSTALL_EXT_URL+"?extensionId=ext-id&extensionVersion=ver&dbHost=host&dbPort=8563", "", 500) - suite.Regexp(`{"code":500,"message":"Internal server error",.*`, responseString) + suite.isInternalServerError(responseString, mockError) } // Upgrade extension @@ -171,9 +173,9 @@ func (suite *RestAPISuite) TestUpgradeExtensionsSuccessfully() { } func (suite *RestAPISuite) TestUpgradeExtensionsFailsWithGenericError() { - suite.controller.On("UpgradeExtension", mock.Anything, mock.Anything, "ext-id").Return(nil, fmt.Errorf("mock error")) + suite.controller.On("UpgradeExtension", mock.Anything, mock.Anything, "ext-id").Return(nil, mockError) responseString := suite.makeRequest("POST", UPGRADE_EXT_URL+"?extensionId=ext-id&extensionVersion=ver&dbHost=host&dbPort=8563", "", 500) - suite.Regexp(`{"code":500,"message":"Internal server error",.*`, responseString) + suite.isInternalServerError(responseString, mockError) } func (suite *RestAPISuite) TestUpgradeExtensionsFailsWithAPIError() { @@ -204,10 +206,10 @@ func (suite *RestAPISuite) TestCreateInstanceFailedInvalidPayload() { } func (suite *RestAPISuite) TestCreateInstanceFailed() { - suite.controller.On("CreateInstance", mock.Anything, mock.Anything, "ext-id", "ext-version", []extensionController.ParameterValue{{Name: "p1", Value: "v1"}}).Return(nil, fmt.Errorf("mock error")) + suite.controller.On("CreateInstance", mock.Anything, mock.Anything, "ext-id", "ext-version", []extensionController.ParameterValue{{Name: "p1", Value: "v1"}}).Return(nil, mockError) responseString := suite.makeRequest("POST", CREATE_INSTANCE_URL+VALID_DB_ARGS, `{"parameterValues": [{"name":"p1", "value":"v1"}]}`, 500) - suite.Regexp(`{"code":500,"message":"Internal server error",.*`, responseString) + suite.isInternalServerError(responseString, mockError) } // List instances @@ -223,9 +225,9 @@ func (suite *RestAPISuite) TestListInstancesSuccessfully() { } func (suite *RestAPISuite) TestListInstancesFailedGenericError() { - suite.controller.On("FindInstances", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(nil, fmt.Errorf("mock")) + suite.controller.On("FindInstances", mock.Anything, mock.Anything, "ext-id", "ext-version").Return(nil, mockError) responseString := suite.restApi.makeRequestWithAuthHeader("GET", LIST_INSTANCES_URL+VALID_DB_ARGS, "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==", "", 500) - suite.Contains(responseString, "{\"code\":500,\"message\":\"Internal server error\"") + suite.isInternalServerError(responseString, mockError) } func (suite *RestAPISuite) TestListInstancesFailedApiError() { @@ -247,9 +249,9 @@ func (suite *RestAPISuite) TestDeleteInstanceSuccessfully() { } func (suite *RestAPISuite) TestDeleteInstanceFailedGenericError() { - suite.controller.On("DeleteInstance", mock.Anything, mock.Anything, "ext-id", "ext-version", "inst-id").Return(fmt.Errorf("mock")) + suite.controller.On("DeleteInstance", mock.Anything, mock.Anything, "ext-id", "ext-version", "inst-id").Return(mockError) responseString := suite.restApi.makeRequestWithAuthHeader("DELETE", DELETE_INSTANCE_URL+VALID_DB_ARGS, "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==", "", 500) - suite.Contains(responseString, "{\"code\":500,\"message\":\"Internal server error\"") + suite.isInternalServerError(responseString, mockError) } func (suite *RestAPISuite) TestDeleteInstanceFailedApiError() { @@ -312,3 +314,7 @@ func (suite *RestAPISuite) makeRequest(method, path, body string, expectedStatus authHeader := createBasicAuthHeader("user", "password") return suite.restApi.makeRequestWithAuthHeader(method, path, authHeader, body, expectedStatus) } + +func (suite *RestAPISuite) isInternalServerError(response string, expectedCause error) { + suite.Contains(response, fmt.Sprintf(`{"code":500,"message":"Internal server error: %s",`, expectedCause.Error())) +} diff --git a/pkg/restAPI/standaloneApi.go b/pkg/restAPI/standaloneApi.go index ee0e2bab..b5819414 100644 --- a/pkg/restAPI/standaloneApi.go +++ b/pkg/restAPI/standaloneApi.go @@ -19,7 +19,7 @@ import ( ) /* [impl -> dsn~rest-interface~1]. */ -func setupStandaloneAPI(controller extensionController.TransactionController) (http.Handler, *openapi.API, error) { +func setupStandaloneAPI(controller extensionController.TransactionController, addCauseToInternalServerError bool) (http.Handler, *openapi.API, error) { api, err := CreateOpenApi() if err != nil { return nil, nil, err @@ -29,7 +29,7 @@ func setupStandaloneAPI(controller extensionController.TransactionController) (h r.Use(loggerMiddleware()) r.Use(middleware.Recoverer) - err = addPublicEndpointsWithController(api, controller) + err = addPublicEndpointsWithController(api, addCauseToInternalServerError, controller) if err != nil { return nil, nil, err }