Skip to content

fix(events): type completed event returnvalue as deserialized value - #4441

Open
toufiq-dev wants to merge 3 commits into
taskforcesh:masterfrom
toufiq-dev:fix/queue-events-completed-returnvalue-type
Open

fix(events): type completed event returnvalue as deserialized value#4441
toufiq-dev wants to merge 3 commits into
taskforcesh:masterfrom
toufiq-dev:fix/queue-events-completed-returnvalue-type

Conversation

@toufiq-dev

Copy link
Copy Markdown

Summary

Closes #4147

This PR fixes the completed event typing on QueueEvents. At runtime, queue-events.ts deserializes the job return value with JSON.parse before emitting it to listeners, so the returnvalue received by event handlers is the actual processor return value (object, number, boolean, etc.) — not the string it was previously typed as.

Changes

  • Make QueueEventsListener generic with a ReturnType = any type parameter (mirroring the existing MinimalJob<DataType, ReturnType> / WorkerListener conventions).
  • Type the completed event as args: { jobId: string; returnvalue: ReturnType; prev?: string }.
  • Make the QueueEvents class generic (QueueEvents<ReturnType = any>) and thread the type parameter through emit/off/on/once.
  • Update the JSDoc for completed to reflect that returnvalue is deserialized from JSON.
  • Update the NestJS guide docs examples that still showed returnvalue: string.

Usage

const queueEvents = new QueueEvents(queueName);

queueEvents.on<QueueEventsListener<{ result: number }>, "completed">(
  "completed",
  ({ returnvalue }) => {
    // returnvalue is now typed as { result: number }
  },
);

Tests

  • Added a regression test in tests/events.test.ts (emits completed global event with the deserialized return value) that processes a job returning an object and asserts the completed event delivers the deserialized object.
  • All 17 tests in tests/events.test.ts pass.
  • tsc, eslint, and prettier are clean.

@manast
manast requested review from Copilot, manast and roggervalf and removed request for roggervalf July 31, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR corrects the TypeScript typing for the QueueEvents "completed" event so that returnvalue is typed as the deserialized processor return value (matching the runtime behavior where queue-events.ts does JSON.parse before emitting).

Changes:

  • Made QueueEventsListener generic (<ReturnType = any>) and updated the "completed" event signature to use ReturnType.
  • Made QueueEvents generic (QueueEvents<ReturnType = any>) and threaded the type parameter through emit/on/once/off.
  • Added a regression test to assert "completed" emits an object return value as an object (not a string), and updated NestJS docs examples accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/events.test.ts Adds a regression test verifying the "completed" event delivers a deserialized (object) returnvalue.
src/classes/queue-events.ts Updates listener and class typings to make "completed".returnvalue generic and aligned with runtime JSON parsing.
docs/gitbook/guide/nestjs/queue-events-listeners.md Updates NestJS guide example to avoid claiming returnvalue is a string.
docs/gitbook/bullmq-pro/nestjs/queue-events-listeners.md Same documentation correction for the BullMQ Pro NestJS guide.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/gitbook/guide/nestjs/queue-events-listeners.md Outdated
@toufiq-dev

Copy link
Copy Markdown
Author

Heads up on CI: the single failing check (testing node@lts/-1, redis@7-alpine) is an unrelated timeout in tests/job_cancellation_advanced.test.ts > should cancel job that makes external API calls (test timed out in 10000ms). That test file is untouched by this PR (this PR only touches src/classes/queue-events.ts, tests/events.test.ts, and two NestJS docs pages). All other jobs — including the main node@lts/* ioredis coverage run, smoke, node-redis, bun, valkey, dragonflydb, and postgres — passed.

@manast
manast force-pushed the fix/queue-events-completed-returnvalue-type branch from 94e9f0b to 255c2bd Compare July 31, 2026 23:03
@toufiq-dev
toufiq-dev requested a review from manast August 1, 2026 07:46

@manast manast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot can you verify if this change also needs to be ported to the other runtimes? Python, Elixir, Rust and PHP?

@manast
manast force-pushed the fix/queue-events-completed-returnvalue-type branch from 255c2bd to d91f756 Compare August 1, 2026 12:36
@toufiq-dev

Copy link
Copy Markdown
Author

Thanks for the review @manast! I checked all four runtimes in the monorepo to verify whether the same issue needs porting:

  • Python (python/bullmq/queue_events.py): already deserializes completed.returnvalue before emitting to listeners via _JSON_DECODE_FIELDS = {"progress": "data", "completed": "returnvalue"} + json.loads(...). Since Python is dynamically typed there is no static typing bug to fix — the runtime behavior already matches the corrected Node contract. No port needed.

  • Rust (rust/src/queue_events.rs): QueueEvent::Completed { return_value: String } deliberately keeps the raw JSON-encoded string (documented as "the raw (JSON-encoded) return value string", and the parses_completed_event test asserts the raw string). The typing here is honest — it really is a string at runtime — so it does not suffer from the "typed as string but actually deserialized" mismatch. Optionally parsing it into serde_json::Value would be a nice parity improvement, but that is a separate enhancement rather than the same bug.

  • Elixir (elixir/lib/bullmq/queue_events.ex): handle_event(:completed, %{job_id: id, returnvalue: value}) passes the raw string through without JSON decoding, and the typespec (optional(:returnvalue) => String.t()) matches the actual runtime value. Again no typing mismatch — a decode-parity follow-up could be considered separately.

  • PHP (php/src/): there is no QueueEvents implementation in the PHP runtime at all (only Job, Queue, Scripts, etc.), so there is nothing to port.

So my take: this fix is correctly Node/TypeScript-only. The other runtimes either already deserialize (Python) or genuinely deliver a string with matching types (Rust/Elixir), and PHP has no global events API yet. Happy to open follow-up issues for Rust/Elixir parse parity if you would like.

@manast
manast force-pushed the fix/queue-events-completed-returnvalue-type branch from d91f756 to 8dc6a10 Compare August 1, 2026 22:11
@manast
manast force-pushed the fix/queue-events-completed-returnvalue-type branch from 8dc6a10 to 2b47467 Compare August 1, 2026 22:36
@toufiq-dev

Copy link
Copy Markdown
Author

Hi @manast quick heads-up: the 9 CI workflows on this PR are sitting in 'awaiting approval' (they were triggered when the branch was last pushed, but never ran). Since it's a fork PR, only a maintainer can approve them. Could you 'Approve and run' on the checks when you get a chance? Once they pass, it should be ready to merge. Thanks!

@manast
manast force-pushed the fix/queue-events-completed-returnvalue-type branch 3 times, most recently from 841f143 to 55c82c9 Compare August 4, 2026 21:51
@manast
manast force-pushed the fix/queue-events-completed-returnvalue-type branch from 55c82c9 to 00f9da1 Compare August 5, 2026 10:16
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]: QueueEventsListener completed event returnvalue incorrectly typed as string

3 participants