Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix null prototypes breaking the logger #618

Merged
merged 9 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions integration-tests/worker/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

### Patch Changes

- Updated dependencies [2fde0ad]
- Updated dependencies [2fde0ad]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- Updated dependencies [4f5f1dd]
- Updated dependencies [58e0d11]
- Updated dependencies [58e0d11]
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# @openfn/cli

## 1.1.1

### Patch Changes

- Updated dependencies [2fde0ad]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
## 1.1.0

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
7 changes: 7 additions & 0 deletions packages/compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# @openfn/compiler

## 0.0.41

### Patch Changes

- Updated dependencies [2fde0ad]
- @openfn/[email protected]

## 0.0.40

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
7 changes: 7 additions & 0 deletions packages/deploy/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# @openfn/deploy

## 0.4.3

### Patch Changes

- Updated dependencies [2fde0ad]
- @openfn/[email protected]

## 0.4.2

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/deploy/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/deploy",
"version": "0.4.2",
"version": "0.4.3",
"description": "Deploy projects to Lightning instances",
"type": "module",
"exports": {
Expand Down
10 changes: 10 additions & 0 deletions packages/engine-multi/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# engine-multi

## 1.1.1

### Patch Changes

- 2fde0ad: Slightly better error reporting for exceptions
- Updated dependencies [2fde0ad]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
## 1.1.0

### Minor Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/engine-multi/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/engine-multi/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
13 changes: 13 additions & 0 deletions packages/engine-multi/src/worker/thread/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
};
Expand All @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions packages/lightning-mock/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

### Patch Changes

- Updated dependencies [2fde0ad]
- Updated dependencies [2fde0ad]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- Updated dependencies [4f5f1dd]
- Updated dependencies [58e0d11]
- @openfn/[email protected]
Expand Down
6 changes: 6 additions & 0 deletions packages/logger/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/logger/package.json
Original file line number Diff line number Diff line change
@@ -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]>",
Expand Down
14 changes: 10 additions & 4 deletions packages/logger/src/sanitize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -81,6 +86,7 @@ const sanitize = (item: any, options: SanitizeOptions = {}) => {
}
return maybeStringify(obj);
}

return item;
};

Expand Down
10 changes: 10 additions & 0 deletions packages/logger/test/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions packages/logger/test/mock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
19 changes: 19 additions & 0 deletions packages/logger/test/sanitize.test.ts
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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);
Expand Down Expand Up @@ -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');
Expand Down
6 changes: 6 additions & 0 deletions packages/runtime/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# @openfn/runtime

## 1.1.1

### Patch Changes

- Updated dependencies [2fde0ad]
- @openfn/[email protected]
## 1.1.0

### Minor Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/runtime",
"version": "1.1.0",
"version": "1.1.1",
"description": "Job processing runtime.",
"type": "module",
"exports": {
Expand Down
15 changes: 7 additions & 8 deletions packages/ws-worker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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/[email protected]
- @openfn/[email protected]
- Updated dependencies [2fde0ad]
- Updated dependencies [2fde0ad]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]
- @openfn/[email protected]

## 1.0.0

Expand Down
2 changes: 1 addition & 1 deletion packages/ws-worker/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
Loading