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.

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

Feat standardize err msgs #77

Merged
merged 39 commits into from
Dec 19, 2023
Merged

Conversation

cicoyle
Copy link
Contributor

@cicoyle cicoyle commented Nov 30, 2023

Description

Standardizing error codes thorough out Dapr with this errors pkg.

NOTE: I added the gRPC err_detail type to show up in the json output in the same manner as @type since that is how gRPC does it and to not conflict with the field type for PreconditionFailure_Violation. Also, putting the @ sign puts the type as the first field in the field output ordering.

I will update the README example once I finalize my dapr runtime code in a followup PR.

image
image

Issue reference

https://github.com/dapr/proposals/blob/main/0009-BCIRS-error-handling-codes.md

Checklist

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

  • Code compiles correctly
  • Created/updated tests

@cicoyle cicoyle requested review from a team as code owners November 30, 2023 01:17
@cicoyle cicoyle force-pushed the feat-standardize-err-msgs branch from 049fe63 to b420732 Compare November 30, 2023 01:18
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
ItalyPaleAle and others added 22 commits December 11, 2023 14:45
* Move dapr/concurrency to kit

Does not include any code change

Signed-off-by: ItalyPaleAle <[email protected]>

* Fixed copyright year

Signed-off-by: ItalyPaleAle <[email protected]>

* Improved memory usage in error collection

Signed-off-by: ItalyPaleAle <[email protected]>

---------

Signed-off-by: ItalyPaleAle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
No code changes

Signed-off-by: ItalyPaleAle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
* Move dapr/utils/streams to kit

No code changes

Signed-off-by: ItalyPaleAle <[email protected]>

* 💄

Signed-off-by: ItalyPaleAle <[email protected]>

* Lint

Signed-off-by: ItalyPaleAle <[email protected]>

---------

Signed-off-by: ItalyPaleAle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
* Migrate metadata decoder from components-contrib to kit

Required creating the `utils` package for utils.IsTruthy too (ported from runtime)

Signed-off-by: ItalyPaleAle <[email protected]>

* Lint

Signed-off-by: ItalyPaleAle <[email protected]>

---------

Signed-off-by: ItalyPaleAle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
* Update fswatcher to use /events/batcher

Signed-off-by: joshvanl <[email protected]>

* Linting

Signed-off-by: joshvanl <[email protected]>

* Linting

Signed-off-by: joshvanl <[email protected]>

* Add sleep to wait for windows fsnotify to become ready

Signed-off-by: joshvanl <[email protected]>

* Increase time for event to be received to 1 second

Signed-off-by: joshvanl <[email protected]>

---------

Signed-off-by: joshvanl <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
@cicoyle cicoyle force-pushed the feat-standardize-err-msgs branch from 5e5d02f to 8ec73ae Compare December 11, 2023 20:45
errors/messages.go Outdated Show resolved Hide resolved
@artursouza
Copy link
Member

LGTM, just a small change.

Signed-off-by: Artur Souza <[email protected]>
artursouza
artursouza previously approved these changes Dec 14, 2023
errors/errors.go Show resolved Hide resolved
errors/messages.go Outdated Show resolved Hide resolved
errors/errors.go Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Outdated Show resolved Hide resolved
errors/errors.go Show resolved Hide resolved
cicoyle and others added 2 commits December 15, 2023 12:48
Signed-off-by: Cassandra Coyle <[email protected]>
Co-authored-by: Alessandro (Ale) Segala <[email protected]>
Signed-off-by: Cassie Coyle <[email protected]>
@cicoyle cicoyle requested a review from ItalyPaleAle December 15, 2023 19:00
errors/errors_test.go Outdated Show resolved Hide resolved
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
errors/errors.go Outdated Show resolved Hide resolved
Signed-off-by: Cassandra Coyle <[email protected]>
@cicoyle cicoyle requested a review from ItalyPaleAle December 18, 2023 21:17
Copy link
Contributor

@JoshVanL JoshVanL left a comment

Choose a reason for hiding this comment

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

Awesome work 😄

@ItalyPaleAle ItalyPaleAle merged commit fd317d2 into dapr:main Dec 19, 2023
6 checks passed
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.

6 participants