From 5e318af37d1f8f1a68930b7efdd197e8ebf0423d Mon Sep 17 00:00:00 2001 From: Xavier Redondo Bolet Date: Thu, 4 Apr 2024 16:18:36 +0200 Subject: [PATCH] feat(bautajs-core): update tap behaviour to make it consistent (#139) --- docs/decorators/tap.md | 148 ++++++++++- .../bautajs-core/src/decorators/pipeline.ts | 2 +- packages/bautajs-core/src/decorators/tap.ts | 250 +++++++++++++++--- .../src/decorators/test/tap.test.ts | 135 +++++++++- 4 files changed, 483 insertions(+), 52 deletions(-) diff --git a/docs/decorators/tap.md b/docs/decorators/tap.md index 13d2b992..1ddec9d4 100644 --- a/docs/decorators/tap.md +++ b/docs/decorators/tap.md @@ -1,7 +1,96 @@ -# tap decorator +# `tap` decorator Decorator that allows to transparently perform actions or side-effects, such as logging without loosing the previous step result. You can explore the source code [here](../../packages/bautajs-core/src/decorators/tap.ts). +## `tap` decorator flow + +The `tap` decorator accepts any number of step functions and executes them sequentially. + +The `tap` decorator behaviour in a few words: + +If all goes well (no errors), at the end of the `tap` decorator execution, you will get as the result of this decorator the value returned by the previous step of this decorator (or in other words, the first element received by the first step function of the `tap` decorator). + +If there is an error, the error will be managed by the error handler attached to the decorator (be it the default one or a custom set through the `catchError` function). This can end in two cases: +- the error handler throws an error --> then this error is thrown by the `tap` decorator +- the custom error handler does not throw any error --> then, as in the happy path case, the `tap` decorator returns as the result of this decorator step function the same value returned by the previous step to this decorator. + +### Short summary of `tap` behaviour + +At the end of `tap` decorator you will get: + +- an error that has interrupted the step function flow +- the same value that was received by the first step function of the `tap` decorator if no error was triggered and thrown. + + +### the flow inside `tap` is sequential as a normal pipeline + +While at the end of the step functions decorated by `tap` you get the input value of the first step, the execution INSIDE `tap` follows the step function pattern in which the values are passed between steps. + +```js +export function thisIsAGoodPipeline() { + return pipe( + stepFunctionReturnsAAA, + tap( + stepFunctionReturnsBBB, // --> input parameter of stepFunctionReturnsBBB is AAA + stepFunctionReturnsCCC, // --> input parameter of stepFunctionReturnsCCC is BBB (not AAA) + stepFunctionReturnsDDD // --> input parameter of stepFunctionReturnsDDD is CCC (not AAA) + ), + stepFunctionReturnsEEE // --> input parameter of stepFunctionReturnsEEE is AAA (CCC is lost) + ); +} + + +``` + +### Beware nesting step functions inside tap + +The `tap` decorator is useful to process side effects knowing that at the end you will get the origin value, but it can make readibility hard if you compose a lot of nested functions inside it. Our advice is to create steps accordingly and call them from the decorator to improve readibility. + +```javascript +// do this +export function thisIsAGoodPipeline() { + return pipe( + firstStepFunction, + secondStepFunction, + tap(sideEffectValidationStepFunction), + thirdStepFunction + ); +} + +// avoid this +export function thisIsABadPipeline() { + return pipe( + firstStepFunction, + secondStepFunction, + tap(async(prev, ctx, bautajs) => { + // Code to access database + // Code to validate certain rules + // Code to decide whether the rules are meet or not and possibly throw an error + }), + thirdStepFunction + ); +} + +``` + +### error handling and custom error handling limitations + +The `tap` decorator allows for a custom error handler. If you do not provide any, the default behaviour is just throw the error through the decorator. + +Two considerations are important if you decide to provide a custom error handling: + +- First: the error handling function must be synchronous. + +- Second: you may ignore any error inside tap through your custom error handling function but the value returned nevertheless will always be the input of the first step function of the decorator, *regardless* of what value you may put in this custom error handler. This is because `tap` deals only with side effects inside their step functions. + +## `tap` decorator usage + +There are two use cases where this decorator is useful: + +1. Synchronous Logging without need to return the previous value +2. Asynchronous validation without need to drag the value between steps: this simplifies the pattern of usage because you do not need to worry about maintaining the value that you want to use *after* the validation or have to worry about mappings like when using `pairwise`. + + ## Example ### 1. Do a console log @@ -11,19 +100,54 @@ Decorator that allows to transparently perform actions or side-effects, such as const randomPreviousStep = step(() => 'I am so random!'); - const pipeline = pipe( - randomPreviousStep, - tap((prev) => { - console.log(`some intermediate step. Prev is ${prev}`); + const sideEffectStep = (prev) => { + console.log(`some intermediate step. Prev is ${prev}`); + + return 'this value will be lost'; + }; - return 'this value will be lost'; - }), - (prev) => { - // prev will be always the result of randomPreviousStep no matter what the tap function returns - console.log(prev); - } - ); + const pipeline = pipe( + randomPreviousStep, + tap(sideEffectStep), + (prev) => { + // prev will be the result of randomPreviousStep + console.log(prev); + } + ); // => 'some intermediate step. Prev is I am so random!' // => 'I am so random!' ``` + +### 2. Asynchronous validation + + +```javascript + const { tap, step, pipe } = require('@axa/bautajs-core'); + + const generateAnObjectToStore = step(() => 'I am so random!'); + + // This is asyncrhonous because this validation requires database or datasource access + const validateThatTheObjectIsCool = step(async (prev, ctx, bautajs) => { + + // database access + + if (theObjectIsNotCool) { + throw new Error('Do not save uncool objects'); + } + }); + + + + const pipeline = pipe( + generateAnObjectToStore, + tap(validateThatTheObjectIsCool), + storeTheObject + ); + +// => case 1. Error throw inside tap --> we get the error and storeTheObject is never called + +// => case 2. No Error thrown from tap --> storeTheObject has the value generated in generateAnObjectToStore, not the undefined returned by validateThatTheObjectIsCool +``` + + diff --git a/packages/bautajs-core/src/decorators/pipeline.ts b/packages/bautajs-core/src/decorators/pipeline.ts index 5acce8c6..058705e8 100644 --- a/packages/bautajs-core/src/decorators/pipeline.ts +++ b/packages/bautajs-core/src/decorators/pipeline.ts @@ -1,7 +1,7 @@ import { BautaJSInstance, Context, GenericError, Pipeline } from '../types'; import { isPromise } from '../utils/is-promise'; -function compose( +export function compose( f1: Pipeline.StepFunction, f2: Pipeline.StepFunction ): Pipeline.StepFunction { diff --git a/packages/bautajs-core/src/decorators/tap.ts b/packages/bautajs-core/src/decorators/tap.ts index 6c7fbc15..2edfb7c7 100644 --- a/packages/bautajs-core/src/decorators/tap.ts +++ b/packages/bautajs-core/src/decorators/tap.ts @@ -1,51 +1,225 @@ -import { Pipeline } from '../types'; +import { GenericError, Pipeline, Context, BautaJSInstance } from '../types'; import { isPromise } from '../utils/is-promise'; +import { compose } from './pipeline'; + +const defaultErrorHandler: Pipeline.CatchError = e => { + throw e; +}; + +export function tap( + f1: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction, + f5: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap< + ValueType, + ResultValue1, + ResultValue2, + ResultValue3, + ResultValue4, + ResultValue5, + ReturnType +>( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction, + f5: Pipeline.StepFunction, + f6: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap< + ValueType, + ResultValue1, + ResultValue2, + ResultValue3, + ResultValue4, + ResultValue5, + ResultValue6, + ReturnType +>( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction, + f5: Pipeline.StepFunction, + f6: Pipeline.StepFunction, + f7: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap< + ValueType, + ResultValue1, + ResultValue2, + ResultValue3, + ResultValue4, + ResultValue5, + ResultValue6, + ResultValue7, + ReturnType +>( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction, + f5: Pipeline.StepFunction, + f6: Pipeline.StepFunction, + f7: Pipeline.StepFunction, + f8: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap< + ValueType, + ResultValue1, + ResultValue2, + ResultValue3, + ResultValue4, + ResultValue5, + ResultValue6, + ResultValue7, + ResultValue8, + ReturnType +>( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction, + f5: Pipeline.StepFunction, + f6: Pipeline.StepFunction, + f7: Pipeline.StepFunction, + f8: Pipeline.StepFunction, + f9: Pipeline.StepFunction +): Pipeline.PipelineFunction; + +export function tap< + ValueType, + ResultValue1, + ResultValue2, + ResultValue3, + ResultValue4, + ResultValue5, + ResultValue6, + ResultValue7, + ResultValue8, + ResultValue9, + ReturnType +>( + f1: Pipeline.StepFunction, + f2: Pipeline.StepFunction, + f3: Pipeline.StepFunction, + f4: Pipeline.StepFunction, + f5: Pipeline.StepFunction, + f6: Pipeline.StepFunction, + f7: Pipeline.StepFunction, + f8: Pipeline.StepFunction, + f9: Pipeline.StepFunction, + f10: Pipeline.StepFunction +): Pipeline.PipelineFunction; /** - * Decorator that allows to transparently perform actions or side-effects, - * such as logging and return the previous step result. - * - * Tap supports promises, but it will ignore any error thrown inside the tapped - * promise and will wait for its execution just to ignore its value. - * - * The step inside tap does not need to return anything, since if it returns anything, - * it will be ignored anyways in favour of what was in the previous step. + * Decorator that allows to transparently perform actions or side-effects + * without losing the initial value that triggered them. * - * @param {Pipeline.StepFunction} stepFn - The step fn to execute - * @returns {Pipeline.StepFunction} + * All the step functions inside the tap pass the value as in a pipeline, but + * the result of the tap will be the value of the previous step. * - * @example + * If an error occurs in the flow inside the tap, the flow is interrupted and goes + * to the error handler. This is to support asynchronous validations inside tap. * - * const { tap, step, pipe } require('@axa/bautajs-core'); + * It is important to note that tap does not parallelize any execution of the promises + * inside the decorator. The execution will have to finish before the next step. * - * const randomPreviousPipeline = step(() => 'I am so random!'); + * @param {stepFunctions: Array>} stepFunctions - The step functions to execute + * @returns {Pipeline.StepFunction} * - * const pipeline = pipe( - * randomPreviousPipeline, - * tap((prev) => { - * console.log(`some intermediate step. Prev is "${prev}""`); - * // prints 'some intermediate step. Prev is "I am so random!"' - * }), - * tap(async (prev) => { - * await asyncProcess(); - * //'whether asyncProcess resolves or rejects. Prev still will be "I am so random!"' - * }), - * (prev) => { - * console.log(prev); - * // prints 'I am so random!' - * } - * ); * */ -export function tap( - stepFn: Pipeline.StepFunction -): Pipeline.StepFunction { - return (prev, ctx, bautajs) => { - const result = stepFn(prev, ctx, bautajs); - - if (isPromise(result)) { - return (result as Promise).then(() => prev).catch(() => prev); + +export function tap( + ...stepFunctions: Array> +): Pipeline.StepFunction { + if (stepFunctions.length === 0 || !stepFunctions.every(fn => typeof fn === 'function')) { + throw new Error('All tap inputs must be a function or a promise.'); + } + + // Merge all step functions in one composited step function + const composition: Pipeline.StepFunction = stepFunctions.reduce( + (acc: Pipeline.StepFunction | undefined, fn) => { + // First iteration will always return the first function since the default value is undefined + if (!acc) { + return fn; + } + // eslint-disable-next-line no-param-reassign + acc = compose(acc, fn); + + return acc; + }, + undefined + ) as Pipeline.StepFunction; + + let errorHandler: Pipeline.CatchError = defaultErrorHandler; + + function tapStepFunction(prev: T, ctx: Context, bautaJS: BautaJSInstance): PromiseLike | T { + try { + const result = composition(prev, ctx, bautaJS); + + if (isPromise(result)) { + // If it is a promise we either return the value of the previous step or we interrup the tap flow if an error has occurred + return (result as Promise) + .then(() => prev) // tap behaviour is ignoring the results of its steps and return the result value of the previous step + .catch((e: GenericError) => { + // This pattern is to ensure that when the error handler throws we throw, and if the error handler *does not* throw, + // tap behaviour is maintained and we return the result value of the previous step + errorHandler(e, ctx, bautaJS) as any; + return prev; + }); + } + + return prev; + } catch (e: any) { + // This pattern is to ensure that when the error handler throws we throw, and if the error handler *does not* throw, + // tap behaviour is maintained and we return the result value of the previous step + errorHandler(e, ctx, bautaJS) as any; + return prev; + } + } + + tapStepFunction.catchError = function catchError( + fn: Pipeline.CatchError + ): Pipeline.StepFunction { + // Even when through casting we can bypass the typescript type check we do not allow non-functions as this parameter + if (typeof fn !== 'function') { + throw new Error('Tap catchError function must be called with a function or a promise.'); } - return prev; + errorHandler = fn; + + return tapStepFunction; }; + + return tapStepFunction; } diff --git a/packages/bautajs-core/src/decorators/test/tap.test.ts b/packages/bautajs-core/src/decorators/test/tap.test.ts index da1116f8..14ceaa75 100644 --- a/packages/bautajs-core/src/decorators/test/tap.test.ts +++ b/packages/bautajs-core/src/decorators/test/tap.test.ts @@ -53,7 +53,7 @@ describe('tap decorator', () => { expect(log).toHaveBeenCalledWith('star wars'); }); - test('should ignore the error inside tap and return the previous step value', async () => { + test('should throw the error inside tap', async () => { const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); const tappedPromise = async (name: string) => { @@ -68,8 +68,141 @@ describe('tap decorator', () => { tap(step<{ name: string }, { name: string }>(async ({ name }) => tappedPromise(name))) ); + await expect(() => pipeline({}, createContext({}), {} as BautaJSInstance)).rejects.toThrow( + new Error('We have an error: star wars') + ); + }); + + test('should work if we mix multiple sync and async steps', async () => { + const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + const log = jest.fn(); + const tappedPromise = async (name: string) => { + await delay(100); + return log(name); + }; + + const tappedSyncFunction = (name: string) => `${name} is a great movie!`; + + const getMovie = step(() => ({ name: 'star wars' })); + + const pipeline = pipe( + getMovie, + tap( + step<{ name: string }, string>(({ name }) => name), + tappedSyncFunction, + tappedPromise + ) + ); + + await expect(pipeline({}, createContext({}), {} as BautaJSInstance)).resolves.toStrictEqual({ + name: 'star wars' + }); + + expect(log).toHaveBeenCalledWith('star wars is a great movie!'); + }); + + test('should work with a custom error handler', async () => { + const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + + const getMovie = step(() => ({ name: 'star wars' })); + + const log = jest.fn(); + + const asyncValidationPromise = async (name: string) => { + await delay(100); + + if (name.length > 10) { + throw new Error(`Name ${name} is too long`); + } + }; + + const tappedSyncFunction = (name: string) => `${name} is a great movie!`; + + const customErrorHandler = (e: Error) => { + log(e.message); + throw e; + }; + + const pipeline = pipe( + getMovie, + tap( + step<{ name: string }, string>(({ name }) => name), + tappedSyncFunction, + asyncValidationPromise + ).catchError(customErrorHandler) + ); + + await expect(pipeline({}, createContext({}), {} as BautaJSInstance)).rejects.toThrow( + new Error('Name star wars is a great movie! is too long') + ); + + expect(log).toHaveBeenCalledWith('Name star wars is a great movie! is too long'); + }); + + test('should not allow an empty custom error handler', async () => { + const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + + const getMovie = step(() => ({ name: 'star wars' })); + + const asyncValidationPromise = async (name: string) => { + await delay(100); + + if (name.length > 10) { + throw new Error(`Name ${name} is too long`); + } + }; + + const tappedSyncFunction = (name: string) => `${name} is a great movie!`; + + const customErrorHandler = undefined; + + expect(() => + pipe( + getMovie, + tap( + step<{ name: string }, string>(({ name }) => name), + tappedSyncFunction, + asyncValidationPromise + ).catchError(customErrorHandler as any) + ) + ).toThrow(new Error('Tap catchError function must be called with a function or a promise.')); + }); + + test('should return previous value when the custom error handler does not throw an error', async () => { + const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + + const getMovie = step(() => ({ name: 'star wars' })); + + const log = jest.fn(); + + const asyncValidationPromise = async (name: string) => { + await delay(100); + + if (name.length > 10) { + throw new Error(`Name ${name} is too long`); + } + }; + + const tappedSyncFunction = (name: string) => `${name} is a great movie!`; + + const customErrorHandler = (e: Error) => { + log(e.message); + return 'there is an error but we have decided to ignore it!'; + }; + + const pipeline = pipe( + getMovie, + tap( + step<{ name: string }, string>(({ name }) => name), + tappedSyncFunction, + asyncValidationPromise + ).catchError(customErrorHandler) + ); + await expect(pipeline({}, createContext({}), {} as BautaJSInstance)).resolves.toStrictEqual({ name: 'star wars' }); + + expect(log).toHaveBeenCalledWith('Name star wars is a great movie! is too long'); }); });