Skip to content

fix: stop masking workflow list errors as pre-v1.17#1682

Open
MyMirelHub wants to merge 1 commit into
dapr:masterfrom
MyMirelHub:fix/cli-fallback-errors
Open

fix: stop masking workflow list errors as pre-v1.17#1682
MyMirelHub wants to merge 1 commit into
dapr:masterfrom
MyMirelHub:fix/cli-fallback-errors

Conversation

@MyMirelHub

@MyMirelHub MyMirelHub commented Jul 22, 2026

Copy link
Copy Markdown

Description

dapr workflow list treated any codes.Unknown error from the sidecar as "runtime is pre v1.17" and fell back to the direct state-store path, printing:

Error: connection string is required for all database drivers for Dapr pre v1.17

Unknown is gRPC's catch-all for server-side errors, so every runtime failure was misreported as a version problem and the real error was discarded.

Classifying the error reliably is fragile — a pre-1.17 daprd returns a version-dependent proxy error rather than Unimplemented. Instead, on any listing failure the CLI now prints the RPC error as a warning, attempts the state-store fallback, and reports both errors if the fallback also fails.

Issue reference

Fixes #1681

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation / Not needed

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 workflow dclient’s pre-v1.17 fallback behavior so that only codes.Unimplemented triggers the state-store fallback, while all other gRPC failures (notably codes.Unknown) are surfaced to the user with their real underlying error details.

Changes:

  • Restrict the pre-v1.17 state-store fallback in InstanceIDs and InstanceHistory to codes.Unimplemented only.
  • Add unit tests validating that InstanceIDs surfaces runtime handler failures and still falls back on Unimplemented.
  • Add clarifying inline comments around the fallback decision logic.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
pkg/workflow/dclient/dclient.go Tightens fallback gating to codes.Unimplemented for instance listing/history calls.
pkg/workflow/dclient/dclient_test.go Adds coverage for InstanceIDs error/fallback behavior using a fake gRPC sidecar server.

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

Comment thread pkg/workflow/dclient/dclient.go Outdated
Comment thread pkg/workflow/dclient/dclient_test.go
Comment thread pkg/workflow/dclient/dclient.go Outdated

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

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

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread pkg/workflow/dclient/dclient.go Outdated

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

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

Comments suppressed due to low confidence (1)

pkg/workflow/dclient/dclient.go:231

  • preV117ProxyErrPrefix is documented as a stable prefix, but the check uses strings.Contains, which is looser than intended and could misclassify unrelated codes.Unknown errors that happen to include that substring. Using strings.HasPrefix both matches the constant name/comment and reduces the risk of re-masking genuine runtime errors.
	switch code.Code() {
	case codes.Unimplemented:
		return true
	case codes.Unknown:
		return strings.Contains(code.Message(), preV117ProxyErrPrefix)
	default:

@MyMirelHub MyMirelHub changed the title fix: stop masking workflow list/history errors as pre-v1.17 fix: stop masking workflow list errors as pre-v1.17 Jul 23, 2026
@MyMirelHub
MyMirelHub force-pushed the fix/cli-fallback-errors branch from e995870 to 5398348 Compare July 23, 2026 10:37
@MyMirelHub
MyMirelHub requested a review from Copilot July 23, 2026 10:42

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

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

Comment thread pkg/workflow/dclient/dclient.go
Comment thread pkg/workflow/dclient/dclient_test.go
The pre-v1.17 state-store fallback in InstanceIDs triggered on
codes.Unknown as well as codes.Unimplemented. Unknown is gRPC's
catch-all for server-side errors, so every runtime failure of
ListInstanceIDs was misreported as 'connection string is required for
all database drivers for Dapr pre v1.17' and the underlying error was
discarded.

Classifying the error reliably is fragile (a real pre-1.17 daprd
returns a version-dependent proxy error rather than Unimplemented), so
instead of classifying: print the RPC error as a warning, attempt the
state-store fallback, and if that also fails report both errors.

Signed-off-by: MyMirelHub <15373565+MyMirelHub@users.noreply.github.com>

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

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

@MyMirelHub
MyMirelHub marked this pull request as ready for review July 23, 2026 11:03
@MyMirelHub
MyMirelHub requested review from a team as code owners July 23, 2026 11:03
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.

dapr workflow list misreports any sidecar error as "Dapr pre v1.17" and discards the error message

2 participants