fix(queue): carry the target namespace from enqueue through to delivery - #622
fix(queue): carry the target namespace from enqueue through to delivery#622anthonyiscoding wants to merge 1 commit into
Conversation
An enqueue addressed to a namespace failed outright and, once accepted, was delivered to the wrong namespace. Three separate drops: - `EnqueueInput` is `deny_unknown_fields` and had no `namespace`, so the namespace the engine sends made every namespaced enqueue fail with `unknown field 'namespace'`. - The job did not persist the namespace, so it could not survive a restart: added to the builtin on-disk envelope (`serde(default)`, so jobs written by older workers still deserialize), the RabbitMQ headers on publish and consume, the redis envelope, and `QueueMessage`. - `Invoker` was namespace-blind, so deliveries always landed in the worker's own namespace. Added `call_in_namespace`, defaulting to the existing namespace-blind call so test and embedded invokers are unaffected, and overrode it in `IiiInvoker` to address the target via the SDK's `TriggerRequest::namespace()`. `function_available` needs no change: `engine::functions::info` already resolves a bare `function_id` across namespaces. Verified with the full worker suite (130 tests, including the RabbitMQ e2e tests) and the smoke-tests `ns_queue_dispatch` scenario, which went from SKIP to 2/2 in python, node, and rust; `upgrade_queue_builtin_drain` and `upgrade_queue_rabbitmq_breaking` stay green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 49 skipped (no docs/).
Four for four. Nicely done. |
1 participant
Problem
An enqueue addressed to a namespace never reached its handler.
smoke-tests/ns_queue_dispatch— a worker injobsregisterstask::run, the driver enqueues an invocation addressed tojobs— was silently reportingSKIP: no enqueue providereven with the queue worker running andengine::queue::enqueueregistered.The engine was doing the right thing throughout: it sends the target namespace to the enqueue provider and resolves that provider fine. The worker dropped the namespace in three places.
Fixes
EnqueueInputis#[serde(deny_unknown_fields)]and had nonamespace, so every namespaced enqueue failed withunknown field 'namespace', expected one of queue, function_id, data, messageReceiptId, traceparent, baggage, _caller_worker_id. The scenario's SKIP branch matches any error containing "enqueue", which is what disguised this as a missing worker.#[serde(default)], so jobs written by older workers still deserialize), the RabbitMQ AMQP headers on both publish and consume, the redis envelope, andQueueMessage.Invokerwas namespace-blind, so deliveries landed in the worker's own namespace. Addedcall_in_namespace, defaulting to the existing namespace-blindcall_with_timeoutso the test and embedded invokers need no plumbing, and overrode it inIiiInvokerto target the namespace via the SDK'sTriggerRequest::namespace().function_availableneeded no change —engine::functions::infoalready resolves a barefunction_idacross namespaces and reports the namespace it found.Testing
ns_queue_dispatch: SKIP → 2/2 passed in python, node, and rust.upgrade_queue_builtin_drain,upgrade_queue_rabbitmq_breaking,upgrade_state_survives: still green.Notes for review
publish_to_function_queueis now 11 positional parameters. It was already over the line at 10 (hence the#[allow(clippy::too_many_arguments)]); I added the parameter rather than widen the diff into a signature refactor, but bundling these into aFunctionJobstruct would be a good follow-up.🤖 Generated with Claude Code