Do not make assumption in routes for error instances

Issue: ARSN-441
scality · Nov 26, 2024 · bb1dd25 · bb1dd25
1 parent 732cfa8
commit bb1dd25
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 12 deletions.
diff --git a/lib/s3routes/routes/routeWebsite.ts b/lib/s3routes/routes/routeWebsite.ts
@@ -30,7 +30,7 @@ export default function routerWebsite(
                 // request being redirected
                 if (redirectInfo) {
                     if (err && redirectInfo.withError) {
-                        return routesUtils.redirectRequestOnError(err as ArsenalError,
+                        return routesUtils.redirectRequestOnError(err,
                             'GET', redirectInfo, dataGetInfo, dataRetrievalParams,
                             response, resMetaHeaders, log)
                     }
@@ -65,7 +65,7 @@ export default function routerWebsite(
                 routesUtils.statsReport500(err, statsClient);
                 if (redirectInfo) {
                     if (err && redirectInfo.withError) {
-                        return routesUtils.redirectRequestOnError(err as ArsenalError,
+                        return routesUtils.redirectRequestOnError(err,
                             'HEAD', redirectInfo, null, dataRetrievalParams,
                             response, resMetaHeaders, log)
                     }

diff --git a/lib/s3routes/routesUtils.ts b/lib/s3routes/routesUtils.ts
@@ -904,7 +904,7 @@ export function redirectRequest(
  * @param log - Werelogs instance
  */
 export function redirectRequestOnError(
-    err: ArsenalError,
+    err: ArsenalError | Error,
     method: 'HEAD' | 'GET',
     routingInfo: {
         withError: true;
@@ -918,17 +918,24 @@ export function redirectRequestOnError(
 ) {
     response.setHeader('Location', routingInfo.location);
 
-    if (!dataLocations && err.is.Found) {
+    let error: ArsenalError;
+    if (err instanceof ArsenalError) {
+        error = err;
+    } else {
+        error = errors.InternalError.customizeDescription(err.message);
+    }
+
+    if (!dataLocations && error.is.Found) {
         if (method === 'HEAD') {
-            return errorHeaderResponse(err, response, corsHeaders, log);
+            return errorHeaderResponse(error, response, corsHeaders, log);
         }
         response.setHeader('x-amz-error-code', err.message);
-        response.setHeader('x-amz-error-message', err.description);
+        response.setHeader('x-amz-error-message', error.description);
         return errorHtmlResponse(err, false, '', response, corsHeaders, log);
     }
 
     // This is reached only for website error document (GET only)
-    const overrideErrorCode = err.flatten();
+    const overrideErrorCode = error.flatten();
     overrideErrorCode.code = 301;
     return streamUserErrorPage(ArsenalError.unflatten(overrideErrorCode)!,
         dataLocations || [], retrieveDataParams, response, corsHeaders, log);

diff --git a/tests/unit/s3routes/routeDELETE.spec.js b/tests/unit/s3routes/routeDELETE.spec.js
@@ -107,7 +107,7 @@ describe('routeDELETE', () => {
         );
     });
 
-    it('should return 204 when objectDelete encounters errors other than NoSuchKey or NoSuchVersion', () => {
+    it('should return error code when objectDelete encounters non-arsenal errors', () => {
         request.objectKey = 'objectKey';
         request.query = {};
 
@@ -119,10 +119,7 @@ describe('routeDELETE', () => {
         routeDELETE(request, response, api, log, statsClient);
 
         expect(routesUtils.responseNoBody).toHaveBeenCalledWith(
-            null, {}, response, 204, log,
-        );
-        expect(routesUtils.statsReport500).toHaveBeenCalledWith(
-            otherError, statsClient,
+            otherError, {}, response, undefined, log,
         );
     });
 });