Merge pull request #618 from OpenFn/fix/616-serialize-errors

Fix null prototypes breaking the logger
OpenFn · Mar 4, 2024 · 220afbd · 220afbd
2 parents 6b4a5fa + c4c1344
commit 220afbd
Show file tree

Hide file tree

Showing 23 changed files with 226 additions and 19 deletions.
diff --git a/integration-tests/worker/CHANGELOG.md b/integration-tests/worker/CHANGELOG.md
@@ -4,6 +4,12 @@
 
 ### Patch Changes
 
+- Updated dependencies [2fde0ad]
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+  - @openfn/engine-multi@1.0.1
+  - @openfn/lightning-mock@2.0.1
+  - @openfn/ws-worker@1.0.1
 - Updated dependencies [4f5f1dd]
 - Updated dependencies [58e0d11]
 - Updated dependencies [58e0d11]

diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md
@@ -1,5 +1,14 @@
 # @openfn/cli
 
+## 1.1.1
+
+### Patch Changes
+
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+  - @openfn/compiler@0.0.41
+  - @openfn/deploy@0.4.3
+  - @openfn/runtime@1.0.1
 ## 1.1.0
 
 ### Patch Changes

diff --git a/packages/cli/package.json b/packages/cli/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/cli",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "description": "CLI devtools for the openfn toolchain.",
   "engines": {
     "node": ">=18",

diff --git a/packages/compiler/CHANGELOG.md b/packages/compiler/CHANGELOG.md
@@ -1,5 +1,12 @@
 # @openfn/compiler
 
+## 0.0.41
+
+### Patch Changes
+
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+
 ## 0.0.40
 
 ### Patch Changes

diff --git a/packages/compiler/package.json b/packages/compiler/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/compiler",
-  "version": "0.0.40",
+  "version": "0.0.41",
   "description": "Compiler and language tooling for openfn jobs.",
   "author": "Open Function Group <[email protected]>",
   "license": "ISC",

diff --git a/packages/deploy/CHANGELOG.md b/packages/deploy/CHANGELOG.md
@@ -1,5 +1,12 @@
 # @openfn/deploy
 
+## 0.4.3
+
+### Patch Changes
+
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+
 ## 0.4.2
 
 ### Patch Changes

diff --git a/packages/deploy/package.json b/packages/deploy/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/deploy",
-  "version": "0.4.2",
+  "version": "0.4.3",
   "description": "Deploy projects to Lightning instances",
   "type": "module",
   "exports": {

diff --git a/packages/engine-multi/CHANGELOG.md b/packages/engine-multi/CHANGELOG.md
@@ -1,5 +1,15 @@
 # engine-multi
 
+## 1.1.1
+
+### Patch Changes
+
+- 2fde0ad: Slightly better error reporting for exceptions
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+  - @openfn/compiler@0.0.41
+  - @openfn/lexicon@1.0.0
+  - @openfn/runtime@1.0.1
 ## 1.1.0
 
 ### Minor Changes

diff --git a/packages/engine-multi/package.json b/packages/engine-multi/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/engine-multi",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "description": "Multi-process runtime engine",
   "main": "dist/index.js",
   "type": "module",

diff --git a/packages/engine-multi/src/errors.ts b/packages/engine-multi/src/errors.ts
@@ -33,11 +33,13 @@ export class ExecutionError extends EngineError {
   message;
 
   original: any; // this is the original error
+
   constructor(original: any) {
     super();
     this.original = original;
 
     this.message = original.message;
+    this.stack = original.stack;
   }
 }
 

diff --git a/packages/engine-multi/src/worker/thread/helpers.ts b/packages/engine-multi/src/worker/thread/helpers.ts
@@ -74,6 +74,7 @@ export const execute = async (
       workflowId,
       // Map the error out of the thread in a serializable format
       error: serializeError(err),
+      stack: err?.stack
       // TODO job id maybe
     });
   };
@@ -89,6 +90,18 @@ export const execute = async (
   // Note that if this occurs after the execute promise resolved,
   // it'll be ignored (ie the workerEmit call will fail)
   process.on('uncaughtException', async (err: any) => {
+    // Log this error to local stdout. This won't be sent out of the worker thread.
+    console.debug(`Uncaught exception in worker thread (workflow ${workflowId} )`)
+    console.debug(err)
+
+    // Also try and log to the workflow's logger
+    try {
+      console.error(`Uncaught exception in worker thread (workflow ${workflowId} )`)
+      console.error(err)
+    } catch(e) {
+      console.error(`Uncaught exception in worker thread`)
+    }
+
     // For now, we'll write this off as a crash-level generic execution error
     // TODO did this come from job or adaptor code?
     const e = new ExecutionError(err);

diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md
@@ -4,6 +4,12 @@
 
 ### Patch Changes
 
+- Updated dependencies [2fde0ad]
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+  - @openfn/engine-multi@1.0.1
+  - @openfn/lexicon@1.0.0
+  - @openfn/runtime@1.0.1
 - Updated dependencies [4f5f1dd]
 - Updated dependencies [58e0d11]
   - @openfn/engine-multi@1.1.0

diff --git a/packages/logger/CHANGELOG.md b/packages/logger/CHANGELOG.md
@@ -1,5 +1,11 @@
 # @openfn/logger
 
+## 1.0.1
+
+### Patch Changes
+
+- 2fde0ad: Don't blow up if an object with a null prototype is sent through
+
 ## 1.0.0
 
 ### Major Changes

diff --git a/packages/logger/package.json b/packages/logger/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/logger",
-  "version": "1.0.0",
+  "version": "1.0.1",
   "description": "Cross-package logging utility",
   "module": "dist/index.js",
   "author": "Open Function Group <[email protected]>",

diff --git a/packages/logger/src/sanitize.ts b/packages/logger/src/sanitize.ts
@@ -49,6 +49,12 @@ const sanitize = (item: any, options: SanitizeOptions = {}) => {
   const maybeStringify = (o: any) =>
     options.stringify === false ? o : stringify(o, undefined, 2);
 
+  // Ignore primitive values
+  if (/^(number|string|boolean)$/.exec(typeof item)) {
+    return item;
+  }
+
+  // Serialize errors
   if (isError(item)) {
     if (options.serializeErrors) {
       return {
@@ -62,10 +68,9 @@ const sanitize = (item: any, options: SanitizeOptions = {}) => {
 
   if (options.policy?.match(/^(remove|obfuscate|summarize)$/)) {
     return scrubbers[options.policy](item);
-  } else if (
-    Array.isArray(item) ||
-    (isNaN(item) && item && typeof item !== 'string')
-  ) {
+  } else if (Array.isArray(item)) {
+    return maybeStringify(item);
+  } else if (item) {
     const obj = item as Record<string, unknown>;
     if (obj && obj.configuration) {
       // This looks sensitive, so let's sanitize it
@@ -81,6 +86,7 @@ const sanitize = (item: any, options: SanitizeOptions = {}) => {
     }
     return maybeStringify(obj);
   }
+
   return item;
 };
 

diff --git a/packages/logger/test/logger.test.ts b/packages/logger/test/logger.test.ts
@@ -107,6 +107,16 @@ test('do log null after a string', (t) => {
   t.is(logger._history.length, 1);
 });
 
+test("log objects with null prototype", (t) => {
+  const logger = createLogger(undefined, { level: 'debug' });
+
+  const obj = Object.create(null)
+  logger.log(obj);
+
+  t.is(logger._history.length, 1);
+});
+
+
 test('sanitize: remove object', (t) => {
   const logger = createLogger(undefined, {
     level: 'debug',

diff --git a/packages/logger/test/mock.test.ts b/packages/logger/test/mock.test.ts
@@ -99,6 +99,7 @@ test('_parse raw message', (t) => {
   logger.success('x', 1, true);
 
   const { messageRaw } = logger._parse(logger._last);
+
   t.is(messageRaw[0], 'x');
   t.is(messageRaw[1], 1);
   t.true(messageRaw[2]);

diff --git a/packages/logger/test/sanitize.test.ts b/packages/logger/test/sanitize.test.ts
@@ -1,6 +1,7 @@
 import test from 'ava';
 
 import sanitize, { SECRET } from '../src/sanitize';
+
 test('simply return a string', (t) => {
   const result = sanitize('x');
   t.is(result, 'x');
@@ -16,6 +17,17 @@ test('simply return a number', (t) => {
   t.true(result === 0);
 });
 
+
+test('simply return true', (t) => {
+  const result = sanitize(true);
+  t.true(result);
+});
+
+test('simply return false', (t) => {
+  const result = sanitize(false);
+  t.false(result);
+});
+
 test('simply return undefined', (t) => {
   const result = sanitize(undefined);
   t.deepEqual(result, undefined);
@@ -105,6 +117,13 @@ test('preserve top level stuff after sanitizing', (t) => {
   t.deepEqual(json, expectedState);
 });
 
+test("don't blow up on null prototypes", (t) => {
+  const obj = Object.create(null)
+  const result = sanitize(obj);
+
+  t.deepEqual(result, '{}');
+});
+
 test('ignore a string with obfuscation', (t) => {
   const result = sanitize('x', { policy: 'obfuscate' });
   t.is(result, 'x');

diff --git a/packages/runtime/CHANGELOG.md b/packages/runtime/CHANGELOG.md
@@ -1,5 +1,11 @@
 # @openfn/runtime
 
+## 1.1.1
+
+### Patch Changes
+
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
 ## 1.1.0
 
 ### Minor Changes

diff --git a/packages/runtime/package.json b/packages/runtime/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/runtime",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "description": "Job processing runtime.",
   "type": "module",
   "exports": {

diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md
@@ -1,16 +1,15 @@
 # ws-worker
 
-## 1.1.0
-
-Allow runs to use multiple versions of the same adaptor
+## 1.1.1
 
 ### Patch Changes
 
-- 58e0d11: Move version log to workflow start
-- Updated dependencies [4f5f1dd]
-- Updated dependencies [58e0d11]
-  - @openfn/engine-multi@1.1.0
-  - @openfn/runtime@1.1.0
+- Updated dependencies [2fde0ad]
+- Updated dependencies [2fde0ad]
+  - @openfn/logger@1.0.1
+  - @openfn/engine-multi@1.0.1
+  - @openfn/lexicon@1.0.0
+  - @openfn/runtime@1.0.1
 
 ## 1.0.0
 

diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfn/ws-worker",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "description": "A Websocket Worker to connect Lightning to a Runtime Engine",
   "main": "dist/index.js",
   "type": "module",