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

Add failure_message to ctx.actions.run #18555

Open
ismell opened this issue Jun 1, 2023 · 5 comments
Open

Add failure_message to ctx.actions.run #18555

ismell opened this issue Jun 1, 2023 · 5 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request

Comments

@ismell
Copy link

ismell commented Jun 1, 2023

Description of the feature request:

ctx.actions.run currently has a progress_message that gets printed when the action is running. I would like a failure_message that will get printed if the action fails. I want to output a set of debug instructions so the user can fix their issue. I could pass in a failure message into my action, but I don't want it depending on that input since it won't affect the output.

What underlying problem are you trying to solve with this feature?

Make debugging failures easier.

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

6.2.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@sgowroji sgowroji added the team-Configurability Issues for Configurability team label Jun 3, 2023
@aiuto aiuto added team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts and removed team-Configurability Issues for Configurability team labels Jun 12, 2023
@comius comius added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Aug 22, 2023
@comius
Copy link
Contributor

comius commented Aug 22, 2023

P4 - because we should be conservative with new features / actions shouldn't be designed to fail

cc @lberki who did a recent review on a PR that adds additional logging to the C++ actions and might think such a FR is P3

@tbaing
Copy link
Contributor

tbaing commented Sep 1, 2023

This isn't a case of "designed to fail" so much as "things will fail sometimes, and it's designed to have a good user experience when that does happen." What we'd imagine would be for the error message for whatever target fails to say "To debug this failure, you can execute bazel run ${FAILED_TARGET}_debug." That message references an _debug target we generate to accompany each normal target, and is specific to whichever target actually failed (which might not be the one you invoked from the command line, because it might be a dependency of whatever you invoked).

Not having this means we either need to rely on people knowing the run whichever target failed's _debug companion (which isn't very discoverable, and relies on people having previously learned to do this) or we need to output something generic in response to a failure (e.g. "To debug your failure, you can take the target name of whatever target failed, append _debug and bazel run that."), which is more discoverable but makes people actually assemble the command to run it. Whereas the line in the first paragraph can be copied and pasted and is discoverable.

@lberki
Copy link
Contributor

lberki commented Sep 4, 2023

I feel similarly to #19302 : this looks like a reasonable feature for a build to have, but I don't see why it needs to be a feature of Bazel: the action is under your control, so it's possible to wrap the action in an if [[ $? != 0 ]]; then echo <debug instructions>; fi; exit $?, which AFAIU does the exact same thing as the feature requested here.

Am I missing something?

@ismell
Copy link
Author

ismell commented Sep 4, 2023

My only reasoning for this FR was that I didn't want to add the message into the action because if we change the message, it will cache bust all the actions. I understand it adds more surface area to the API, so feel free to close if you want.

@lberki
Copy link
Contributor

lberki commented Sep 5, 2023

I started a separate thread about your use case (my mental model is that action cache entries are cheap to regenerate individually, they have value only "in bulk"), but assuming that's the model you settle on, I could imagine something like this:

  1. A "heavyweight" action that does a lot of work, then in addition to its regular outputs, emits another file containing the exit code and maybe another with its console output and which always succeeds
  2. A "lightweight" action that does the actual error reporting and returns the serialized exit code from action (1) as its own exit code and maybe adds some smart diagnostics

This way, you can change (2) without losing the cache entries for (1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants