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

Refactor error handling #820

Merged
merged 50 commits into from
May 17, 2024
Merged

Refactor error handling #820

merged 50 commits into from
May 17, 2024

Conversation

cdavernas
Copy link
Member

@cdavernas cdavernas commented Feb 15, 2024 •
edited
Loading

Many thanks for submitting your Pull Request ❤️!

Please specify parts of this PR update:

  • Specification
  • Schema
  • Examples
  • Extensions
  • Roadmap
  • Use Cases
  • Community
  • TCK
  • Other

Discussion or Issue link:

#681
#770
#771
#772
#569
#698
#731

What this PR does / why we need it:
Refactors the error handling

Closes #681
Closes #770
Closes #771
Closes #772

Additional notes for reviewers:

The PR goes well beyond the scope of the initial issue, or the first drafts we produced in past discussions.

  • Retries can now only be performed in case of handled errors. By definition, that's what a retry is

re-enter a command, especially because an error was made the first time.

-- Oxford Languages

  • Error handling can be performed at both action and state level, so retries are possible for both
  • Removed the autoRetries property which did not make sense with the new approach to retries
  • Terms such as retryableErrors/nonRetryableErrors have been removed in favor of using when/exceptWhen clause. It achieves the exact same, but looks IMHO much cleaner (and cooler).
  • Updated actions by adding the ability to throw an error thanks to the errorRef property

@cdavernas
feat(Specification): Refactor error handling
318bfdc 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Schema): Fixed the errors.json schema
1f7c5ae
@cdavernas
fix(Schema): Fixed the errors.json
c446b75 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Schema): Fixed the errors.json schema
a032cbe 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
Fixed the errors.json schema
9979afd 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Examples): Fixed invalid examples
117eeb0 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Schemas): Fixed the errors.json schema
89d3866 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Schema)
b20ff0b 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Schema)
0844bf5
@cdavernas
fix(Schema)
c113e52 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
feat(Schema): Added "error" operation, used to throw an error
7f7cb70 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas cdavernas added this to the v0.9 milestone Feb 15, 2024
@cdavernas cdavernas self-assigned this Feb 15, 2024
@cdavernas cdavernas added change: feature New feature or request. Impacts in a minor version change area: spec Changes in the Specification examples labels Feb 15, 2024
JBBianchi
JBBianchi reviewed Feb 15, 2024
View reviewed changes
Copy link
Member

@JBBianchi JBBianchi left a comment

Choose a reason for hiding this comment

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

Overall, it looks great. Thanks a lot @cdavernas for this contribution !

specification.md Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Show resolved Hide resolved
@cdavernas
fix: Fixed Schema and Specification to address requested changes
acae83e 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
@cdavernas
fix(Roadmap): Updated the roadmap
0d067d0 
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
ricardozanini
ricardozanini reviewed Feb 21, 2024
View reviewed changes
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

Great work! I'll finish the feature review tomorrow. Today's only content structure. Many thanks, that's amazing!

schema/errors.json Outdated Show resolved Hide resolved
specification.md Show resolved Hide resolved
specification.md Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
specification.md Outdated Show resolved Hide resolved
fjtirado
fjtirado reviewed Feb 22, 2024
View reviewed changes
specification.md Outdated Show resolved Hide resolved
cdavernas and others added 3 commits March 8, 2024 11:56
@cdavernas @ricardozanini
Update schema/errors.json
ff1d1f2 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
a47b636 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
dad2fa0 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
ricardozanini
ricardozanini approved these changes May 17, 2024
View reviewed changes
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

LGTM! Just a side note about accepting the grammar/typos suggestions.

cdavernas and others added 4 commits May 17, 2024 15:12
@cdavernas @ricardozanini
Update specification.md
49013fd 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
26557df 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
2e9aa9b 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
aa7f3df 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
cdavernas and others added 23 commits May 17, 2024 15:14
@cdavernas @ricardozanini
Update specification.md
b215715 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
42cb30a 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
f808d07 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
2c84916 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
fbb7791 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
4dfab1a 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
131207d 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
b823af0 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
112db02 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
f3a3e0c 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
bac05dc 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
1bbf5be 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
68d1126 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
2867d09 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
4bd9fce 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
8cdb169 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
9f9854d 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
ef42822 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
48df490 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
cbca234 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
be7da4b 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
17433bb 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas @ricardozanini
Update specification.md
0073684 
Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
@cdavernas cdavernas merged commit 6e2d402 into serverlessworkflow:main May 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JBBianchi JBBianchi JBBianchi left review comments

@fjtirado fjtirado fjtirado left review comments

@ricardozanini ricardozanini ricardozanini approved these changes

@tsurdilo tsurdilo Awaiting requested review from tsurdilo

@antmendoza antmendoza Awaiting requested review from antmendoza

Assignees

@cdavernas cdavernas

Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Projects
Progress Tracker
Status: Done
Milestone
v0.9.0
Development

Successfully merging this pull request may close these issues.

Refactor retries Add means to throw an error Refactor error Add multi retryRef to action
4 participants
@cdavernas @ricardozanini @fjtirado @JBBianchi