Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Return HTTP 409 Conflict when ETag doesn't match #2619

Closed
amolenk opened this issue Jan 4, 2021 · 3 comments · Fixed by dapr/components-contrib#579 or #2647
Closed

Return HTTP 409 Conflict when ETag doesn't match #2619

amolenk opened this issue Jan 4, 2021 · 3 comments · Fixed by dapr/components-contrib#579 or #2647
Assignees
@yaron2
Labels
breaking-change This is a breaking change P1

Comments

@amolenk
Copy link

amolenk commented Jan 4, 2021

In what area(s)?

/area runtime

Describe the proposal

When encountering an ETag mismatch while trying to update state, Dapr will return an HTTP 500 status code. It would be better to return an HTTP 409 Conflict status code to distinguish from other server failures.

The error code in the HTTP response body is ERR_STATE_SAVE, which is also pretty generic for an ETag conflict.
@amolenk amolenk mentioned this issue Jan 4, 2021
Open
@rynowak
Copy link
Contributor

rynowak commented Jan 4, 2021

Piling on this with a somewhat related bug (gRPC caller in this case): dapr/dotnet-sdk#426

For SDKs and just general sanity we need to make sure it's possible to programmatically tell the difference between an ETag mismatch and other types of state store failure.

@msfussell msfussell added P1 breaking-change This is a breaking change discussion and removed discussion labels Jan 7, 2021
@amolenk amolenk mentioned this issue Jan 8, 2021
Merged
3 tasks
@yaron2 yaron2 self-assigned this Jan 9, 2021
@yaron2 yaron2 mentioned this issue Jan 9, 2021
Merged
@amolenk
Copy link
Author

amolenk commented Jan 9, 2021 •
edited
Loading

@yaron2 @artursouza I don't think this should be closed yet. It was closed by dapr/components-contrib#579, but that PR only partially closes this issue. It ensures that a specialized ETagError is used by all state components, but those errors still need to be translated to HTTP 409 status codes by the runtime.

@yaron2 yaron2 reopened this Jan 9, 2021
@yaron2
Copy link
Member

yaron2 commented Jan 9, 2021

GitHub closed it automatically because it was mentioned

@yaron2 yaron2 mentioned this issue Jan 11, 2021
Merged
rynowak added a commit to rynowak/dapr that referenced this issue Jan 12, 2021
@rynowak
Use Aborted error code for ETag mismatch
2bbf277 
Related to dapr#2619

I started a discussion after dapr#2647 was
merged because I think there's a better fit for an ETag mismatch than
AlreadyExists.

The gRPC docs have specific guidance that Aborted is the best status for
'when a client-specified test-and-set fails', which is this case.
@rynowak rynowak mentioned this issue Jan 12, 2021
Merged
7 tasks
yaron2 pushed a commit that referenced this issue Jan 12, 2021
@rynowak
Use Aborted error code for ETag mismatch (#2648)
4678e47 
Related to #2619

I started a discussion after #2647 was
merged because I think there's a better fit for an ETag mismatch than
AlreadyExists.

The gRPC docs have specific guidance that Aborted is the best status for
'when a client-specified test-and-set fails', which is this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yaron2 yaron2

Labels
breaking-change This is a breaking change P1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Treat etag errors across all state stores
add etag error distinction for state operations
4 participants
@rynowak @amolenk @msfussell @yaron2