chore: Clearer Temporal connection and missing-versioning-behavior errors#769
Merged
Merged
Conversation
Wrap the client Dial error with the target address, the underlying cause, and a readiness hint instead of returning it bare, so a failed 'rr serve' reports the address and reason rather than an opaque 'temporal_plugin_serve: context deadline exceeded'. Ref: roadrunner-server/roadrunner#2278
When worker versioning is enabled (DeploymentOptions.UseVersioning with a deployment version) and no worker default behavior is set, the Temporal SDK panics inside RegisterWorkflowWithOptions if a workflow has no VersioningBehavior, crashing the worker. Validate up front and return a descriptive error naming the workflow and task queue instead. Ref: roadrunner-server/roadrunner#2237
There was a problem hiding this comment.
Pull request overview
This PR improves error clarity in the Temporal plugin in two situations: connection failures and missing workflow versioning behavior. Both fixes extract small pure helpers covered by unit tests.
Changes:
- Wrap
tclient.Dialerrors with target address, underlying cause, and a readiness hint via a newconnectErrorhelper. - Add
validateVersioningBehaviorto pre-empt the Temporal SDK panic when worker versioning is enabled without a default or per-workflow behavior, returning a descriptive error naming the workflow and task queue. - Add unit tests for both helpers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal.go | Wraps Dial errors with the new connectError helper. |
| internal_test.go | Adds unit test for connectError. |
| aggregatedpool/workers.go | Adds validateVersioningBehavior and calls it before RegisterWorkflowWithOptions. |
| aggregatedpool/workers_test.go | Adds unit tests covering versioning validation paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ioning Note that the deprecated UseBuildIDForVersioning flag is intentionally out of scope; the supported DeploymentOptions.UseVersioning path is fully covered.
…he condition RegisterWorkflowWithOptions panics on invalid versioning config rather than returning an error. The previous guard mirrored the SDK's internal panic condition, which is a standing sync obligation against a private internal and drifts silently. Wrap registration in a recover that converts any panic into a clean, workflow-named init error. Correctness no longer depends on any SDK-internal condition or message; the missing-versioning-behavior case still gets a tailored hint and degrades gracefully otherwise. This also covers the deprecated UseBuildIDForVersioning path for free. Ref: roadrunner-server/roadrunner#2237
rustatian
changed the title
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
…t-error-and-versioning
wolfy-j
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two error-clarity fixes in the Temporal plugin.
temporal_plugin_serve: context deadline exceeded— e.g.failed to connect to the Temporal server at "localhost:7233": context deadline exceeded; ensure it is reachable and ready to accept connections.VersioningBehavior) now fails worker init with a clear, workflow-named error instead of crashing the process on an SDK panic.closes roadrunner-server/roadrunner#2278
closes roadrunner-server/roadrunner#2237