Merge pull request #4422 from backstage/rugvip/explicit

backend-common,catalog: be more explicit about response types

.changeset/odd-buckets-compare.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Set explicit content-type in error handler responses.

.changeset/strange-cobras-unite.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Refactored route response handling to use more explicit types and throw errors.

packages/backend-common/src/middleware/errorHandler.ts

@@ -63,10 +63,10 @@ export function errorHandler(
   return (
     error: Error,
     _request: Request,
-    response: Response,
+    res: Response,
     next: NextFunction,
   ) => {
-    if (response.headersSent) {
+    if (res.headersSent) {
       // If the headers have already been sent, do not send the response again
       // as this will throw an error in the backend.
       next(error);
@@ -80,7 +80,9 @@ export function errorHandler(
       logger.error(error);
     }
 
-    response.status(status).send(message);
+    res.status(status);
+    res.setHeader('content-type', 'text/plain');
+    res.send(message);
   };
 }
 

plugins/catalog-backend/src/service/router.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { errorHandler } from '@backstage/backend-common';
+import { errorHandler, NotFoundError } from '@backstage/backend-common';
 import {
   locationSpecSchema,
   analyzeLocationSchema,
@@ -57,7 +57,7 @@ export async function createRouter(
         const filter = EntityFilters.ofQuery(req.query);
         const fieldMapper = translateQueryToFieldMapper(req.query);
         const entities = await entitiesCatalog.entities(filter);
-        res.status(200).send(entities.map(fieldMapper));
+        res.status(200).json(entities.map(fieldMapper));
       })
       .post('/entities', async (req, res) => {
         const body = await requireRequestBody(req);
@@ -67,7 +67,7 @@ export async function createRouter(
         const [entity] = await entitiesCatalog.entities(
           EntityFilters.ofMatchers({ 'metadata.uid': result.entityId }),
         );
-        res.status(200).send(entity);
+        res.status(200).json(entity);
       })
       .get('/entities/by-uid/:uid', async (req, res) => {
         const { uid } = req.params;
@@ -75,15 +75,14 @@ export async function createRouter(
           EntityFilters.ofMatchers({ 'metadata.uid': uid }),
         );
         if (!entities.length) {
-          res.status(404).send(`No entity with uid ${uid}`);
-        } else {
-          res.status(200).send(entities[0]);
+          throw new NotFoundError(`No entity with uid ${uid}`);
         }
+        res.status(200).json(entities[0]);
       })
       .delete('/entities/by-uid/:uid', async (req, res) => {
         const { uid } = req.params;
         await entitiesCatalog.removeEntityByUid(uid);
-        res.status(204).send();
+        res.status(204).end();
       })
       .get('/entities/by-name/:kind/:namespace/:name', async (req, res) => {
         const { kind, namespace, name } = req.params;
@@ -95,14 +94,11 @@ export async function createRouter(
           }),
         );
         if (!entities.length) {
-          res
-            .status(404)
-            .send(
-              `No entity with kind ${kind} namespace ${namespace} name ${name}`,
-            );
-        } else {
-          res.status(200).send(entities[0]);
+          throw new NotFoundError(
+            `No entity with kind ${kind} namespace ${namespace} name ${name}`,
+          );
         }
+        res.status(200).json(entities[0]);
       });
   }
 
@@ -111,7 +107,7 @@ export async function createRouter(
       const input = await validateRequestBody(req, locationSpecSchema);
       const dryRun = yn(req.query.dryRun, { default: false });
       const output = await higherOrderOperation.addLocation(input, { dryRun });
-      res.status(201).send(output);
+      res.status(201).json(output);
     });
   }
 
@@ -119,22 +115,22 @@ export async function createRouter(
     router
       .get('/locations', async (_req, res) => {
         const output = await locationsCatalog.locations();
-        res.status(200).send(output);
+        res.status(200).json(output);
       })
       .get('/locations/:id/history', async (req, res) => {
         const { id } = req.params;
         const output = await locationsCatalog.locationHistory(id);
-        res.status(200).send(output);
+        res.status(200).json(output);
       })
       .get('/locations/:id', async (req, res) => {
         const { id } = req.params;
         const output = await locationsCatalog.location(id);
-        res.status(200).send(output);
+        res.status(200).json(output);
       })
       .delete('/locations/:id', async (req, res) => {
         const { id } = req.params;
         await locationsCatalog.removeLocation(id);
-        res.status(204).send();
+        res.status(204).end();
       });
   }
 
@@ -142,7 +138,7 @@ export async function createRouter(
     router.post('/analyze-location', async (req, res) => {
       const input = await validateRequestBody(req, analyzeLocationSchema);
       const output = await locationAnalyzer.analyzeLocation(input);
-      res.status(200).send(output);
+      res.status(200).json(output);
     });
   }