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

Build not stopped when rethrowing exception in OnError #2661

Closed
GeertvanHorrik opened this issue Nov 15, 2019 · 3 comments · Fixed by #2664
Closed

Build not stopped when rethrowing exception in OnError #2661

GeertvanHorrik opened this issue Nov 15, 2019 · 3 comments · Fixed by #2664
Labels
Bug
Milestone

Comments

@GeertvanHorrik
Copy link
Contributor

@GeertvanHorrik GeertvanHorrik commented Nov 15, 2019

What You Are Seeing?

As described in the docs, I am using OnError to catch an exception and log something.

I do expect the build to fail though when I throw another (same) exception).

What is Expected?

Build to fail and exit code of cake to be != 0.

What version of Cake are you using?

0.35.0

Are you running on a 32 or 64 bit system?

64-bit

What environment are you running on? Windows? Linux? Mac?

Windows

Are you running on a CI Server? If so, which one?

Irrelevant

How Did You Get This To Happen? (Steps to Reproduce)

Here is an example code:

.OnError<BuildContext>(async (ex, buildContext) => 
{
    await buildContext.SourceControl.MarkBuildAsFailedAsync("Build");

    Error(ex);

    throw ex;
});
@GeertvanHorrik

This comment has been minimized.

Copy link
Contributor Author

@GeertvanHorrik GeertvanHorrik commented Nov 15, 2019

I think I found the reason. There are no task overloads in https://github.com/cake-build/cake/blob/develop/src/Cake.Core/CakeTaskBuilder.Errors.cs, so async methods are not allowed.

Would you be open to a PR that adds Func as well?

@patriksvensson

This comment has been minimized.

Copy link
Member

@patriksvensson patriksvensson commented Nov 18, 2019

@GeertvanHorrik Absolutely. I think that async should be supported for this as well.

@cake-build/cake-team Any objections to this?

@devlead

This comment has been minimized.

Copy link
Member

@devlead devlead commented Nov 18, 2019

@cake-build/cake-team Any objections to this?

No objection from me.

GeertvanHorrik added a commit to GeertvanHorrik/cake that referenced this issue Nov 20, 2019
@mholo65 mholo65 added the Bug label Nov 20, 2019
@mholo65 mholo65 added this to the v0.36.0 milestone Nov 20, 2019
GeertvanHorrik added a commit to GeertvanHorrik/cake that referenced this issue Dec 2, 2019
Fix build errors

Fix dry run stuff

Fix unit tests
devlead added a commit that referenced this issue Dec 3, 2019
* GeertvanHorrik-pr/async-on-error:
  Add async/sync OnError integration tests
  (GH-2661) Support async implementations of error handling
@devlead devlead closed this in #2664 Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.