Merge pull request #4287 from apostrophecms/pro-4779-base

Merge hotfix back to main

CHANGELOG.md

@@ -14,7 +14,15 @@ structure. For consistent results, one might also choose to override the `render
 methods of the `@apostrophecms/area` module, which are used to render content while editing.
 Thanks to Michelin for their support of this work.
 
-## 3.55.0
+## 3.55.1 (2023-09-11)
+
+### Fixes
+
+* The structured logging for API routes now responds properly if an API route throws a `string` as an exception, rather than
+a politely `Error`-derived object with a `stack` property. Previously this resulted in an error message about the logging
+system itself, which was not useful for debugging the original exception.
+
+## 3.55.0 (2023-08-30)
 
 ### Adds
 

modules/@apostrophecms/module/index.js

@@ -258,7 +258,7 @@ module.exports = {
             self.logError(req, `api-error${typeTrail}`, msg, {
               name: response.name,
               status: response.code,
-              stack: error.stack.split('\n').slice(1).map(line => line.trim()),
+              stack: (error.stack || '').split('\n').slice(1).map(line => line.trim()),
               errorPath: response.path,
               data: response.data
             });

package.json

@@ -1,6 +1,6 @@
 {
   "name": "apostrophe",
-  "version": "3.55.0",
+  "version": "3.55.1",
   "description": "The Apostrophe Content Management System.",
   "main": "index.js",
   "scripts": {

test/weird-api-errors.js

@@ -0,0 +1,81 @@
+const t = require('../test-lib/test.js');
+const assert = require('assert');
+
+describe('Don\'t crash on weird API errors', function() {
+
+  after(async function() {
+    return t.destroy(apos);
+  });
+
+  this.timeout(t.timeout);
+
+  let apos;
+
+  it('should initialize apos', async function() {
+    apos = await t.create({
+      root: module,
+      modules: {
+        'api-test': {
+          apiRoutes(self) {
+            return {
+              get: {
+                fetchItFine(req) {
+                  return {
+                    nifty: true
+                  };
+                },
+                fetchItFailWeird(req) {
+                  throw 'not-an-error-object';
+                },
+                fetchItFailNormal(req) {
+                  throw new Error('normal error');
+                }
+              }
+            };
+          }
+        }
+      }
+    });
+  });
+  it('should fetch fine in the normal case', async function() {
+    const body = await apos.http.get('/api/v1/api-test/fetch-it-fine', {});
+    assert(typeof body === 'object');
+    assert.strictEqual(body.nifty, true);
+  });
+  it('should fail politely in the weird case of a non-Error exception', async function() {
+    let msgWas;
+    const consoleError = console.error;
+    console.error = msg => {
+      msgWas = msg;
+    };
+    try {
+      await apos.http.get('/api/v1/api-test/fetch-it-fail-weird', {});
+      // Should not get here
+      assert(false);
+    } catch (e) {
+      // Make sure the logging system itself is not at fault
+      assert(!msgWas.toString().includes('Structured logging error'));
+    } finally {
+      console.error = consoleError;
+      console.error(msgWas);
+    }
+  });
+  it('should fail politely in the normal case of an Error exception', async function() {
+    let msgWas;
+    const consoleError = console.error;
+    console.error = msg => {
+      msgWas = msg;
+    };
+    try {
+      await apos.http.get('/api/v1/api-test/fetch-it-fail-normal', {});
+      // Should not get here
+      assert(false);
+    } catch (e) {
+      // Make sure the logging system itself is not at fault
+      assert(!msgWas.toString().includes('Structured logging error'));
+    } finally {
+      console.error = consoleError;
+      console.error(msgWas);
+    }
+  });
+});