Skip to content

fix(flow-producer): reject deduplication and debounce options (#2780)#4121

Open
mohanrajvenkatesan23-04 wants to merge 1 commit into
taskforcesh:masterfrom
mohanrajvenkatesan23-04:fix/issue-2780-flow-debounce-error
Open

fix(flow-producer): reject deduplication and debounce options (#2780)#4121
mohanrajvenkatesan23-04 wants to merge 1 commit into
taskforcesh:masterfrom
mohanrajvenkatesan23-04:fix/issue-2780-flow-debounce-error

Conversation

@mohanrajvenkatesan23-04
Copy link
Copy Markdown
Contributor

Why

FlowProducer.add() and addBulk() silently dropped opts.deduplication / opts.debounce. A dedup hit returns the existing job's id rather than the freshly generated one, which orphans the flow's children — there is no safe way to reconcile the two ids inside the multi-exec transaction.

Per maintainer @roggervalf on #2780:

"actually this won't work for a root parent either using flow producer because the parent ids should be known in each addition before running the transaction. Dedup logic can retrieve a different id than the one being generated from the producer."
"this functionality is not supported... We should throw an error in this case."

Closes #2780.

What

  • src/classes/flow-producer.ts: file-private assertNoDeduplication(flow) helper called at the top of add() and addBulk(). Throws a clear error naming the offending queue/job.
  • tests/flow.test.ts: three regression tests covering deduplication on add(), debounce on add(), and deduplication mixed into addBulk().

I left the FlowJob type alone for now — removing debounce/deduplication from the root type would be a TypeScript breaking change that warrants a separate decision. The runtime check covers JS users and any TS escape hatch.

Test plan

  • tests/flow.test.ts adds three new cases under "when deduplication or debounce is provided to a flow"
  • CI runs the existing flow test suite against redis@7-alpine, valkey@8, and dragonflydb@latest

FlowProducer cannot honour deduplication or debounce: parent ids must be
known before multi.exec() commits, but a dedup hit returns the existing
job's id rather than the freshly generated one — orphaning the flow's
children. The FlowJob type currently still permits these opts at the
root, and they were silently dropped at runtime.

Throw a clear error from add() and addBulk() instead. Per maintainer
guidance on taskforcesh#2780: "we should throw an error in this case".

- Add assertNoDeduplication() helper in flow-producer.ts
- Call it at the top of add() and addBulk()
- Add three regression tests in tests/flow.test.ts covering both opts
  on add() and the bulk path

Closes taskforcesh#2780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Debounced job ID not being returned on new flow job creation

1 participant