diff --git a/integration-tests/execute/CHANGELOG.md b/integration-tests/execute/CHANGELOG.md index 4568707b6..78f8dd585 100644 --- a/integration-tests/execute/CHANGELOG.md +++ b/integration-tests/execute/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/integration-tests-execute +## 1.0.9 + +### Patch Changes + +- Updated dependencies [1cbbba0] + - @openfn/runtime@1.5.3 + ## 1.0.8 ### Patch Changes diff --git a/integration-tests/execute/package.json b/integration-tests/execute/package.json index cec8424a1..670e38473 100644 --- a/integration-tests/execute/package.json +++ b/integration-tests/execute/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-execute", "private": true, - "version": "1.0.8", + "version": "1.0.9", "description": "Job execution tests", "author": "Open Function Group ", "license": "ISC", diff --git a/integration-tests/worker/CHANGELOG.md b/integration-tests/worker/CHANGELOG.md index 324f17ad0..e71ba43e4 100644 --- a/integration-tests/worker/CHANGELOG.md +++ b/integration-tests/worker/CHANGELOG.md @@ -1,5 +1,13 @@ # @openfn/integration-tests-worker +## 1.0.67 + +### Patch Changes + +- @openfn/engine-multi@1.4.3 +- @openfn/lightning-mock@2.0.24 +- @openfn/ws-worker@1.8.4 + ## 1.0.66 ### Patch Changes diff --git a/integration-tests/worker/package.json b/integration-tests/worker/package.json index 4e669f13f..19abf3db7 100644 --- a/integration-tests/worker/package.json +++ b/integration-tests/worker/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-worker", "private": true, - "version": "1.0.66", + "version": "1.0.67", "description": "Lightning WOrker integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index b591f9000..7b512efbe 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,13 @@ # @openfn/cli +## 1.8.10 + +### Patch Changes + +- Warn when an expression doesn't return state +- Updated dependencies [1cbbba0] + - @openfn/runtime@1.5.3 + ## 1.8.9 ### Patch Changes diff --git a/packages/cli/package.json b/packages/cli/package.json index 23d495429..2dec89ecc 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.8.9", + "version": "1.8.10", "description": "CLI devtools for the openfn toolchain.", "engines": { "node": ">=18", diff --git a/packages/engine-multi/CHANGELOG.md b/packages/engine-multi/CHANGELOG.md index faa2e912a..add173d82 100644 --- a/packages/engine-multi/CHANGELOG.md +++ b/packages/engine-multi/CHANGELOG.md @@ -1,5 +1,12 @@ # engine-multi +## 1.4.3 + +### Patch Changes + +- Updated dependencies [1cbbba0] + - @openfn/runtime@1.5.3 + ## 1.4.2 ### Patch Changes diff --git a/packages/engine-multi/package.json b/packages/engine-multi/package.json index f038538b1..0e192b616 100644 --- a/packages/engine-multi/package.json +++ b/packages/engine-multi/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/engine-multi", - "version": "1.4.2", + "version": "1.4.3", "description": "Multi-process runtime engine", "main": "dist/index.js", "type": "module", diff --git a/packages/engine-multi/test/integration.test.ts b/packages/engine-multi/test/integration.test.ts index bdaf48fb9..a148942d2 100644 --- a/packages/engine-multi/test/integration.test.ts +++ b/packages/engine-multi/test/integration.test.ts @@ -151,19 +151,20 @@ test.serial('trigger workflow-log for job logs', (t) => { let didLog = false; - api.execute(plan, emptyState).on('workflow-log', (evt) => { - if (evt.name === 'JOB') { - didLog = true; - t.deepEqual(evt.message, JSON.stringify(['hola'])); - t.pass('workflow logged'); - } - }); - - api.execute(plan, emptyState).on('workflow-complete', (evt) => { - t.true(didLog); - t.falsy(evt.state.errors); - done(); - }); + api + .execute(plan, emptyState) + .on('workflow-log', (evt) => { + if (evt.name === 'JOB') { + didLog = true; + t.deepEqual(evt.message, JSON.stringify(['hola'])); + t.pass('workflow logged'); + } + }) + .on('workflow-complete', (evt) => { + t.true(didLog); + t.falsy(evt.state.errors); + done(); + }); }); }); diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md index a4ea4b29e..51118fca5 100644 --- a/packages/lightning-mock/CHANGELOG.md +++ b/packages/lightning-mock/CHANGELOG.md @@ -1,5 +1,13 @@ # @openfn/lightning-mock +## 2.0.24 + +### Patch Changes + +- Updated dependencies [1cbbba0] + - @openfn/runtime@1.5.3 + - @openfn/engine-multi@1.4.3 + ## 2.0.23 ### Patch Changes diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json index bae196075..0b8d331d0 100644 --- a/packages/lightning-mock/package.json +++ b/packages/lightning-mock/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/lightning-mock", - "version": "2.0.23", + "version": "2.0.24", "private": true, "description": "A mock Lightning server", "main": "dist/index.js", diff --git a/packages/runtime/CHANGELOG.md b/packages/runtime/CHANGELOG.md index ae92876c1..62be75290 100644 --- a/packages/runtime/CHANGELOG.md +++ b/packages/runtime/CHANGELOG.md @@ -1,5 +1,11 @@ # @openfn/runtime +## 1.5.3 + +### Patch Changes + +- 1cbbba0: warn when an expression doesn't return state + ## 1.5.2 ### Patch Changes diff --git a/packages/runtime/package.json b/packages/runtime/package.json index ed91c93f1..6259c5a51 100644 --- a/packages/runtime/package.json +++ b/packages/runtime/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/runtime", - "version": "1.5.2", + "version": "1.5.3", "description": "Job processing runtime.", "type": "module", "exports": { diff --git a/packages/runtime/src/execute/expression.ts b/packages/runtime/src/execute/expression.ts index 64bc694f7..9b0da2498 100644 --- a/packages/runtime/src/execute/expression.ts +++ b/packages/runtime/src/execute/expression.ts @@ -18,6 +18,11 @@ import { } from '../errors'; import type { JobModule, ExecutionContext } from '../types'; import { ModuleInfoMap } from '../modules/linker'; +import { + clearNullState, + isNullState, + createNullState, +} from '../util/null-state'; export type ExecutionErrorWrapper = { state: any; @@ -105,8 +110,21 @@ export const wrapOperation = ( return async (state: State) => { logger.debug(`Starting operation ${name}`); const start = new Date().getTime(); + if (isNullState(state)) { + clearNullState(state); + logger.warn( + `WARNING: No state was passed into operation ${name}. Did the previous operation return state?` + ); + } const newState = immutableState ? clone(state) : state; - const result = await fn(newState); + + let result = await fn(newState); + + if (!result) { + logger.debug(`Warning: operation ${name} did not return state`); + result = createNullState(); + } + // TODO should we warn if an operation does not return state? // the trick is saying WHICH operation without source mapping const duration = printDuration(new Date().getTime() - start); diff --git a/packages/runtime/src/execute/step.ts b/packages/runtime/src/execute/step.ts index b9c8e732b..3c4f5f8c9 100644 --- a/packages/runtime/src/execute/step.ts +++ b/packages/runtime/src/execute/step.ts @@ -15,7 +15,7 @@ import { NOTIFY_JOB_ERROR, NOTIFY_JOB_START, } from '../events'; -import stringify from 'fast-safe-stringify'; +import { isNullState } from '../util/null-state'; const loadCredentials = async ( job: Job, @@ -80,6 +80,7 @@ const prepareFinalState = ( logger: Logger, statePropsToRemove?: string[] ) => { + if (isNullState(state)) return undefined; if (state) { if (!statePropsToRemove) { // As a strict default, remove the configuration key @@ -94,12 +95,12 @@ const prepareFinalState = ( removedProps.push(prop); } }); - logger.debug( - `Cleaning up state. Removing keys: ${removedProps.join(', ')}` - ); + if (removedProps.length) + logger.debug( + `Cleaning up state. Removing keys: ${removedProps.join(', ')}` + ); - const cleanState = stringify(state); - return JSON.parse(cleanState); + return clone(state); } return state; }; diff --git a/packages/runtime/src/util/null-state.ts b/packages/runtime/src/util/null-state.ts new file mode 100644 index 000000000..71c88449b --- /dev/null +++ b/packages/runtime/src/util/null-state.ts @@ -0,0 +1,18 @@ +// This module manages a special state object with a hidden null symbol. +// Used to track when operations and jobs do not return their own state + +const NULL_STATE = Symbol('null_state'); + +// The good thing about using a Symbol is that even if we forget to clean the object. +// it's still represented as {}, because symbols aren't visible as keys +export function createNullState() { + return { [NULL_STATE]: true }; +} + +export function isNullState(state: any) { + return typeof state === 'object' && state[NULL_STATE] === true; +} + +export function clearNullState(state: any) { + if (typeof state === 'object') delete state[NULL_STATE]; +} diff --git a/packages/runtime/test/execute/expression.test.ts b/packages/runtime/test/execute/expression.test.ts index 1ecd02d5d..85b2105b1 100644 --- a/packages/runtime/test/execute/expression.test.ts +++ b/packages/runtime/test/execute/expression.test.ts @@ -5,6 +5,7 @@ import type { Operation, State } from '@openfn/lexicon'; import execute, { mergeLinkerOptions } from '../../src/execute/expression'; import type { ExecutionContext } from '../../src/types'; +import { isNullState } from '../../src/util/null-state'; type TestState = State & { data: { @@ -139,16 +140,43 @@ test.serial('async operations run in series', async (t) => { t.is(result.data.x, 12); }); -test.serial('jobs can return undefined', async (t) => { +test.serial( + 'jobs return null-state instead of undefined or null', + async (t) => { + // @ts-ignore violating the operation contract here + const job = [() => undefined] as Operation[]; + + const state = createState() as TestState; + const context = createContext(); + + const result = (await execute(context, job, state, {})) as TestState; + + t.assert(isNullState(result)); + } +); + +test.serial('warn when an operation does not return state', async (t) => { // @ts-ignore violating the operation contract here - const job = [() => undefined] as Operation[]; + const job = [ + (s) => s, + () => {}, + (s) => { + s.data = { a: 'a' }; + return s; + }, + ] as Operation[]; const state = createState() as TestState; const context = createContext(); const result = (await execute(context, job, state, {})) as TestState; + t.deepEqual(result, { data: { a: 'a' } }); + + const debugLog = logger._find('debug', /did not return state/); + t.truthy(debugLog); - t.assert(result === undefined); + const warningLog = logger._find('warn', /No state was passed into operation/); + t.truthy(warningLog); }); test.serial('jobs can mutate the original state', async (t) => { diff --git a/packages/runtime/test/security.test.ts b/packages/runtime/test/security.test.ts index 94fe6d81a..bec1d0cbb 100644 --- a/packages/runtime/test/security.test.ts +++ b/packages/runtime/test/security.test.ts @@ -1,7 +1,7 @@ // a suite of tests with various security concerns in mind import test from 'ava'; import { createMockLogger } from '@openfn/logger'; -import type { ExecutionPlan, State } from '@openfn/lexicon'; +import type { ExecutionPlan } from '@openfn/lexicon'; import run from '../src/runtime'; @@ -64,12 +64,12 @@ test.serial( ); test.serial('jobs should not have access to global scope', async (t) => { - const src = 'export default [() => globalThis.x]'; + const src = 'export default [() => ({x: globalThis.x, y: "some-val"})]'; // @ts-ignore globalThis.x = 42; const result: any = await run(src); - t.falsy(result); + t.deepEqual(result, { y: 'some-val' }); // @ts-ignore delete globalThis.x; @@ -90,16 +90,17 @@ test.serial('jobs should be able to mutate global state', async (t) => { }); test.serial('jobs should each run in their own context', async (t) => { - const src1 = 'export default [() => { globalThis.x = 1; return 1;}]'; - const src2 = 'export default [() => globalThis.x]'; + const src1 = + 'export default [() => { globalThis.x = 1; return { x: globalThis.x }}]'; + const src2 = 'export default [() => { return { x: globalThis.x }}]'; await run(src1); const r1 = (await run(src1)) as any; - t.is(r1, 1); + t.deepEqual(r1, { x: 1 }); const r2 = (await run(src2)) as any; - t.is(r2, undefined); + t.deepEqual(r2, {}); }); test.serial('jobs should not have a process object', async (t) => { diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md index ff947a30a..343a05ef2 100644 --- a/packages/ws-worker/CHANGELOG.md +++ b/packages/ws-worker/CHANGELOG.md @@ -1,5 +1,14 @@ # ws-worker +## 1.8.4 + +### Patch Changes + +- Warn when an expression doesn't return state +- Updated dependencies [1cbbba0] + - @openfn/runtime@1.5.3 + - @openfn/engine-multi@1.4.3 + ## 1.8.3 ### Patch Changes diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json index d6bfdbf4b..1f51c2a8f 100644 --- a/packages/ws-worker/package.json +++ b/packages/ws-worker/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/ws-worker", - "version": "1.8.3", + "version": "1.8.4", "description": "A Websocket Worker to connect Lightning to a Runtime Engine", "main": "dist/index.js", "type": "module",