Problem when multiple workflows inherit from base class

[jabstone]

Hello. I ran into a problem recently when you have multiple workflows that inherit from a common base class. In this scenario, calling initializeWorkflows will incorrectly register ALL message handlers (from every workflow) for each workflow. So message handlers from workflow A will also be registered for workflow B and vice versa. You can see this behavior when looking at the results of the getSteps method in the handles.js.

    static getSteps(target) {
        return Reflect.getMetadata(exports.WORKFLOW_HANDLES_METADATA_KEY, target) || [];
    }

The end result is when the registry-handler tries to resolve the handler for a message, it incorrectly tries to send the message to the wrong handler. Eventually, the correct handler will be found and the message will be delivered, assuming you don't exhaust your message delivery retries. It seems the root cause is the Reflection.getMetadata is incorrectly returning handlers for the wrong targets when using inheritance. Either that or I'm just doing something really dumb and haven't figured out what yet. I modified the bus-starter project to demonstrate the issue. It can be found at: https://github.com/jabstone/bus-starter/tree/bug/InheritedWorkflowProblem

Any insight as to possible workarounds for this scenario would be appreciated.

[jabstone]

Here is the error you will see when the registry handler tries to deliver the message to the wrong handler.

2021-03-19T21:53:16.160Z warn: Message was unsuccessfully handled. Returning to queue [object Object] {
"name": "ServiceBus",
"error": {
"name": "TypeError",
"message": "Cannot read property 'bind' of undefined",
"stack": "TypeError: Cannot read property 'bind' of undefined\n at Binding.handler [as dynamicValue] (/home/joe/github/bus-starter/node_modules/@node-ts/bus-workflow/dist/workflow/registry/workflow-registry.js:94:71)\n at /home/joe/github/bus-starter/node_modules/inversify/lib/resolution/resolver.js:64:118\n at invokeFactory (/home/joe/github/bus-starter/node_modules/inversify/lib/resolution/resolver.js:11:16)\n at /home/joe/github/bus-starter/node_modules/inversify/lib/resolution/resolver.js:64:26\n at Object.resolve (/home/joe/github/bus-starter/node_modules/inversify/lib/resolution/resolver.js:97:12)\n at /home/joe/github/bus-starter/node_modules/inversify/lib/container/container.js:321:37\n at Container._get (/home/joe/github/bus-starter/node_modules/inversify/lib/container/container.js:312:44)\n at Container.get (/home/joe/github/bus-starter/node_modules/inversify/lib/container/container.js:232:21)\n at Object.resolveHandler (/home/joe/github/bus-starter/node_modules/@node-ts/bus-core/dist/handler/handler-registry.js:66:38)\n at dispatchMessageToHandler (/home/joe/github/bus-starter/node_modules/@node-ts/bus-core/dist/service-bus/service-bus.js:138:41)"

[jabstone]

Going through the code a bit more it seems the target passed to addStep in handles.ts is always the base class. Even for message handlers that are only defined in the child workflows and not the base workflow. I'm going to just refactor my implementation since this scenario doesn't seem to be supported. :(

    static addStep(metadata, target) {
        reflect_extensions_1.ReflectExtensions.defineMetadata(exports.WORKFLOW_HANDLES_METADATA_KEY, metadata, target.constructor);
    }

[adenhertog]

Hey @jabstone, it looks like you already worked this one out so I don't think there's anything else I can add. I had a look at your fork and as you know the decorator on the base class can only target itself and not what extends it.

You'll know best how to refactor this - but as far as I can tell you'd likely end up along the lines of:

• composing the logic via injectors into each workflow
• continuing to inherit, but add a handler decorator at the concrete class level that calls its base class
• having a separate dedicated child workflow

all the best

[jabstone]

Yep, that's exactly what I ended up doing. Works great now. Appreciate the response.