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

Better handling for GitVersion failure cases #2730

Closed
wants to merge 1 commit into from

Conversation

michaelnoonan
Copy link

GitVersion was failing for our builds this week, but the reason was a mystery until we were able to dig into the GitVersion log file on a failed build. What made this really difficult is the actual error was masked by the GitVersionRunner.

The error according to Cake was Expecting state 'Element'.. Encountered 'Text' with name '', namespace ''. This is accurate because the DataContactJsonSerializer is failing to deserialise what is actually an error message from gitversion.exe. However, it is ultimately misleading since the actual error message is masked by this exception.

We changed our process to call GitVersion twice:

  1. Run GitVersion with OutputType = GitVersionOutput.BuildServer so it logs to stdout, including any error messages
  2. Run GitVersion with OutputType = GitVersionOutput.Json to get the deserialised version information

This PR proposes a similar approach, but within the GitVersionRunner itself so others fall into the pit of success. It appears the intent of the code was to run GitVersion twice, but the deserialization exception prevents the intended behaviour.

Hope this helps!

GitVersion was failing for our builds this week, but the reason was a mystery until we were able to dig into the GitVersion log file on a failed build. What made this really difficult is the actual error was masked by the GitVersionRunner.

The error according to Cake was `Expecting state 'Element'.. Encountered 'Text'  with name '', namespace ''.` This is accurate because the `DataContactJsonSerializer` is failing to deserialise what is actually an error message from `gitversion.exe`. However, it is ultimately misleading since the actual error message is masked by this exception.

We changed our process to call GitVersion twice:

1. Run GitVersion with `OutputType = GitVersionOutput.BuildServer` so it logs to stdout, including any error messages
2. Run GitVersion with `OutputType = GitVersionOutput.Json` to get the deserialised version information

This PR proposes a similar approach, but within the GitVersionRunner itself so others fall into the pit of success. It appears the intent of the code was to run GitVersion twice, but the deserialization exception prevents the intended behaviour.

Hope this helps!
@gitfool
Copy link
Contributor

gitfool commented Mar 16, 2020

@michaelnoonan this should improve slightly now that PR GitTools/GitVersion#2161 has merged and released in GitVersion 5.2.1.

I certainly wouldn't want to call GitVersion twice, especially with noisy build server log output.

I was going to submit a followup PR to Cake that scrapes the ERROR line from the GitVersion when the process exits with non-zero exit code. This would show the relevant error without needing to increase Cake build verbosity or use any other tricks.

@michaelnoonan
Copy link
Author

Cool. I'm happy to leave this with you since it's a concrete example of where we ran into trouble. Calling gitversion twice is not terribly appealing, even though it will likely hit the gitversion caches and be reasonably fast.

@gep13
Copy link
Member

gep13 commented May 13, 2020

@michaelnoonan apologies for not getting back to you sooner about this PR. Was there an issue opened for the work in this PR? Our contributing guidelines mention that an issue should be created first.

@devlead devlead added this to the v0.38.0 milestone May 14, 2020
@devlead
Copy link
Member

devlead commented May 14, 2020

@michaelnoonan thanks for your contribution 👍
I'm going a head and going to close this PR as it should be fixed by #2741, which should be released in Cake 0.38.0.

@cake-build-bot
Copy link

🎉 This issue has been resolved in version v0.38.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

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.

5 participants