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

LC Task Can Fail Without Logging An Error #5912

Open
benvillalobos opened this issue Nov 25, 2020 · 6 comments
Open

LC Task Can Fail Without Logging An Error #5912

benvillalobos opened this issue Nov 25, 2020 · 6 comments

Comments

@benvillalobos
Copy link
Member

Issue Description

Someone reported here that the LC task can fail without throwing a warning.

Some times, when I rebuild my solution (C#) in Visual Studio Enterprise 2019, the following error is shown:


Severity	Code	Description	Project	File	Line	Suppression State

Error		The “LC” task returned false but did not log an error.	SBM


Other times, the solution simply builds without a problem.

ToolTaskExtension may be able to return false and not log an error. Note that the LC task does not implement its own execute function.

@benvillalobos benvillalobos added bug needs-triage Have yet to determine what bucket this goes in. and removed needs-triage Have yet to determine what bucket this goes in. labels Nov 25, 2020
@benvillalobos benvillalobos changed the title LC Task Can Fail Without Throwing a Warning LC Task Can Fail Without Throwing an Error Nov 25, 2020
@benvillalobos benvillalobos added the needs-triage Have yet to determine what bucket this goes in. label Nov 25, 2020
@benvillalobos benvillalobos added this to the MSBuild 16.9 milestone Dec 2, 2020
@rainersigwald rainersigwald added the For consideration Used for items on the backlog to raise them to the top of that list for discussion label Dec 2, 2020
@Forgind
Copy link
Member

Forgind commented Feb 4, 2021

I found something very suspicious:

string pathToTool = ComputePathToTool();
if (pathToTool == null)
{
// An appropriate error should have been logged already.
return false;
}

attempts to calculate a path to the tool to be executed, and if it's still null after the calculation, it returns false without logging an error, claiming an error already should have been logged.

Looking at that function, if UseCommandProcessor is false, and GenerateFullPathToTool (which is implemented by classes that extend ToolTaskExtension) returns null without logging an error, which sounds legit based on

// if we cannot find the file, we'll probably error out later on when
// we try to launch the tool; so do nothing for now
, it will hit this problem.

@Forgind
Copy link
Member

Forgind commented Feb 4, 2021

I think it will also hit this problem if the tool is cancelled:

if (_terminatedTool)
{
return false;
}

returns false without logging an error if _terminatedTool is true.
That's set to true here.
TerminateToolProcess itself doesn't log an error, so if the tool doesn't log an error before being cancelled, that should lead to ToolTask returning false without logging an error.

@rainersigwald rainersigwald modified the milestones: MSBuild 16.9, 17.0 Jun 15, 2021
@marcpopMSFT marcpopMSFT modified the milestones: 17.0, MSBuild 17.1 Jul 9, 2021
@benvillalobos benvillalobos removed the needs-triage Have yet to determine what bucket this goes in. label Jul 29, 2021
@benvillalobos benvillalobos self-assigned this Oct 4, 2021
@benvillalobos benvillalobos changed the title LC Task Can Fail Without Throwing an Error ToolTasks Can Log MSB4181 When Cancelled Oct 14, 2021
@benvillalobos benvillalobos changed the title ToolTasks Can Log MSB4181 When Cancelled LC Task Can Fail Without Logging An Error Oct 14, 2021
@benvillalobos
Copy link
Member Author

Bundling this into what'll be fixed in #6968.

This issue is so vague that the PR could solve the issue. But without repro steps the root cause isn't clear. I'll opt to close this and hope that it's reported next time it happens.

@Forgind
Copy link
Member

Forgind commented Oct 25, 2021

@benvillalobos,
I don't think this has anything to do with #6968. The "returned false without logging an error" error is to indicate a malfunctioning task. You're suggesting disabling the error when cancelled would resolve the issue, but we really just need to log an error here whenever we return false, or else we're just making a malfunctioning task not fail as noisily.

@benvillalobos
Copy link
Member Author

@Forgind My goal here is to close an issue that has no actionable information. It should really have "needs-more-info" applied to it. If you want to keep this open, we can backlog this instead.

@benvillalobos benvillalobos added needs-more-info Issues that need more info to continue investigation. and removed For consideration Used for items on the backlog to raise them to the top of that list for discussion labels Oct 25, 2021
@benvillalobos benvillalobos removed this from the MSBuild 17.1 milestone Oct 25, 2021
@ghost ghost added the stale For issues that haven't had activity in some time. label Nov 25, 2021
@ghost
Copy link

ghost commented Nov 25, 2021

This issue is marked as stale because feedback has been requested for 30 days with no response. Please respond within 14 days or this issue will be closed due to inactivity.

@Forgind Forgind removed the needs-more-info Issues that need more info to continue investigation. label Nov 25, 2021
@Forgind Forgind removed the stale For issues that haven't had activity in some time. label Nov 25, 2021
@Forgind Forgind added this to the Backlog milestone Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants