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

GH-1616: Better error message for interdependent tasks #3142

Merged

Conversation

norpache
Copy link
Contributor

@norpache norpache commented Feb 15, 2021

@dnfadmin
Copy link

dnfadmin commented Feb 15, 2021

CLA assistant check
All CLA requirements met.

@norpache
Copy link
Contributor Author

The case where a task depends on itself could probably also use a better message (Error "Reflexive edges in graph are not allowed."). Let me know if you want it changed in another PR.

@augustoproiete
Copy link
Member

augustoproiete commented Feb 15, 2021

@norpache Thanks for the PR!

I think you're spot on in the sense that we're trying to get a user-friendly message from a low-level API.

Perhaps the way to go about this, would be to throw custom graph exceptions as needed (e.g. CakeGraphCircularReferenceException) from the low-level code including the details that you'd need to craft a user-friendly message (e.g. edge names, etc.) that can bubble up the stack and be caught closer to the UI, and then a CakeException can be thrown with a friendly message, with the custom exception as the innerException.

@cake-build/cake-team What do you think?

@augustoproiete augustoproiete changed the title GH1616 : Better error message for interdependent tasks GH-1616 : Better error message for interdependent tasks Feb 28, 2021
@augustoproiete augustoproiete changed the title GH-1616 : Better error message for interdependent tasks GH-1616: Better error message for interdependent tasks Feb 28, 2021
@p10tyr
Copy link

p10tyr commented Oct 13, 2021

Just migrated everything to Cake.Frosting and got this message.. Was scratching my head but now i know there is a circular dependency... somewhere.. because all tasks are throwing this message.
image

Copy link
Member

@augustoproiete augustoproiete left a comment

Choose a reason for hiding this comment

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

LGTM

@augustoproiete augustoproiete marked this pull request as ready for review October 14, 2021 05:24
And corresponding tests
@augustoproiete augustoproiete force-pushed the better-error-msg-for-circular-ref branch from 48ab263 to 291ddc2 Compare October 14, 2021 13:45
@augustoproiete augustoproiete merged commit 61a7dfb into cake-build:develop Oct 14, 2021
@augustoproiete
Copy link
Member

@norpache your changes have been merged, thanks for your contribution 👍

If you have a Twitter account and want us to give you a shout out when this gets released, let us know your Twitter handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message on circular references leads to poor developer experience
5 participants