Skip to content
New issue

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

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

Already on GitHub? # to your account

The countertop crashes for partially fulfilled input types #149

Closed
slifty opened this issue Mar 4, 2022 · 0 comments · Fixed by #150
Closed

The countertop crashes for partially fulfilled input types #149

slifty opened this issue Mar 4, 2022 · 0 comments · Fixed by #150
Labels
bug Something isn't working

Comments

@slifty
Copy link
Member

slifty commented Mar 4, 2022

Bug

Current Behavior

When setting up a topology that uses an appliance with more than one input type it appears the system fails if an input type is not satisfied.

/Users/slifty/Maestral/Code/tvkitchen/implementations/tvkitchen-newsmax-implementation/node_modules/@tvkitchen/countertop/lib/tools/utils/countertop.js:168
const getStreamTopic = (dataType, stream) => (0, _kafka.sanitizeTopic)(`${dataType}::${stream.id}`);
                                                                                              ^

TypeError: Cannot read properties of undefined (reading 'id')
    at getStreamTopic (/Users/slifty/Maestral/Code/tvkitchen/implementations/tvkitchen-newsmax-implementation/node_modules/@tvkitchen/countertop/lib/tools/utils/countertop.js:168:95)
    at /Users/slifty/Maestral/Code/tvkitchen/implementations/tvkitchen-newsmax-implementation/node_modules/@tvkitchen/countertop/lib/classes/CountertopWorker.js:70:118
    at Array.map (<anonymous>)
    at CountertopWorker.start (/Users/slifty/Maestral/Code/tvkitchen/implementations/tvkitchen-newsmax-implementation/node_modules/@tvkitchen/countertop/lib/classes/CountertopWorker.js:70:70)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Promise.all (index 0)
    at CountertopStation.start (/Users/slifty/Maestral/Code/tvkitchen/implementations/tvkitchen-newsmax-implementation/node_modules/@tvkitchen/countertop/lib/classes/CountertopStation.js:66:24)
    at async Promise.all (index 2)
    at Countertop.start (/Users/slifty/Maestral/Code/tvkitchen/implementations/tvkitchen-newsmax-implementation/node_modules/@tvkitchen/countertop/lib/classes/Countertop.js:66:24)

Expected Behavior

It definitely shouldn't crash, though we face a choice. It could either:

  1. Throw an error if an appliance is added without each input being populated for every appliance.
  2. Deem the topology valid, and whatever data arrives arrives.

I'm inclined to not error; we could potentially create a function that will diagnose whether a topology is "input complete" for users who want that, possibly eventually adding a "strict" setting which would result in an error. For the short term I think having no error is a less abrasive place to start, as there are use cases where an input might be an optional "nice to have" for a given appliance.

@slifty slifty added the bug Something isn't working label Mar 4, 2022
slifty added a commit that referenced this issue Mar 4, 2022
Appliances that take more than one input used to break the topology.
This change makes it so the topology will still run.

Issue #149
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant