-
Notifications
You must be signed in to change notification settings - Fork 905
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
Ensure package folders are correctly restored if unexpected failures occur during upgrade #3489
Comments
The MarkPackagePending method is used to create a .chocolateyPending file in the package folder to indicate that there is a pending operation that needs to be completed. However, during an upgrade operation, the package folder is moved to the lib-bkp folder, and as a result, doesn't exist anymore in the lib folder. If during the upgrade operation, something fails, for example, an exception when trying to download the nupkg file, a process of returning the package folder to the correct place begins. The first thing that is attempted is to create the .chocolateyPending file in the package folder. However, as mentioned, this folder no longer exists in the expected location. As a result, it is possible for an exception to be thrown, as the file cannot be created in the package folder. This problem can lead to another related problem in that the package is now "lost" from lib folder, and will not show up when doing for example choco list. Imagine you were attempting the following command, and an exception happened during the downloading of the nupkg: choco upgrade audacity Chocolatey CLI would start the upgrade operation, and then it would fail downloading the package. It would attempt to create the .chocolateyPending file, and another exception would be thrown. The package folder is not returned to the lib folder, and now the package is "lost". If you attempt to run choco upgrade audacity again, the operation would succeed, as it would see that the package is not installed and it would be installed, however, if this happened during the process of running choco upgrade all, then the underlying audacity application would not be upgraded, as Chocolatey CLI is no longer managing the package. This commit addresses the issue by first checking to see if the package folder exists. If it doesn't, then it doesn't attempt to create the .chocolateyPending file, and simply returns early. This then allows the remaining operations, i.e. returning the package folder from the lib-bkp folder, back to the lib folder.
In order to fully test out the code path where the attempt to write the .chocolateyPending file was failing, we needed to be able to return a 503/504 error from the server. Realistically, this isn't feasible, since it would mean having a server in place, that would be able to respond with the right responses, at the right time. After digging around for a little bit, I found the WireMock.Net project, which seemed to do exactly what was needed, namely: > WireMock.Net is a flexible product for stubbing and mocking web HTTP responses using advanced request matching and response templating. I took this for a spin, and was able to start/stop the server within our test harnesses, and then was able to mock the required HTTP requests, to get to the point during an Upgrade scenario, to then send a 503 response. This then started the code path for the .chocolateyPending file, and I was able to make assertions that the lib/lib-bad/lib-bkp folders worked as expected. The only "change" that was needed to a normal test scenario, was to change this line: Configuration.Sources = "http://localhost:24626/api/v2/"; to force Chocolatey CLI to direct requests to the WireMock.Net server. There is likely LOTS of things that we could start doing with this server, and there is likely some code that needs to be added to encapsulate the creation of the responses, to make them more re-usable, but for now, the required responses for this code path has been hard-coded into the responses. NOTE: There are a LOT of packages added in this commit, and all of them came in as a result of installing the WireMock.Net package. Since this is a test project, i.e. we are not shipping any of these, I don't believe this to be a concern.
@sf0-k46 thank you very much for raising this issue. I have done some investigation on this issue, and I believe I have found out of the root cause of the problem on the Chocolatey CLI side of things (i.e. why the package is "lost" from within the |
In order to fully test out the code path where the attempt to write the .chocolateyPending file was failing, we needed to be able to return a 503/504 error from the server. Realistically, this isn't feasible, since it would mean having a server in place, that would be able to respond with the right responses, at the right time. After digging around for a little bit, I found the WireMock.Net project, which seemed to do exactly what was needed, namely: > WireMock.Net is a flexible product for stubbing and mocking web HTTP responses using advanced request matching and response templating. I took this for a spin, and was able to start/stop the server within our test harnesses, and then was able to mock the required HTTP requests, to get to the point during an Upgrade scenario, to then send a 503 response. This then started the code path for the .chocolateyPending file, and I was able to make assertions that the lib/lib-bad/lib-bkp folders worked as expected. The only "change" that was needed to a normal test scenario, was to change this line: Configuration.Sources = "http://localhost:24626/api/v2/"; to force Chocolatey CLI to direct requests to the WireMock.Net server. There is likely LOTS of things that we could start doing with this server, and there is likely some code that needs to be added to encapsulate the creation of the responses, to make them more re-usable, but for now, the required responses for this code path has been hard-coded into the responses. NOTE: There are a LOT of packages added in this commit, and all of them came in as a result of installing the WireMock.Net package. Since this is a test project, i.e. we are not shipping any of these, I don't believe this to be a concern.
As suggested here: #3500 (comment) By WireMock.Net maintainer. This will hopefully make the Integration Tests work when running on GitHub Actions.
In the previous commit, an attempt was made to remove the usage of the hard-coded WireMock.Net port, however, not all usages of the 24626 port were removed. These have been replaced with .FormatWith(_wireMockServer.Url) where needed, to make the port number dynamic based on what it is started with. This change is an extension of the comment that was left here: #3500 (comment)
The WireMock.Net Server is setup to reply to requests made when _NOT_ using repository optimizations, so let's make sure that we are using that configuration explicitly. It was found that when running all the integration tests together, this value could be set to true elsewhere, which then causes tests here to fail. This is far from ideal, and the Configuration should have been reset when starting this scenario but for now, let's explicitly set to false, as this is what we know the WireMock.Net server will respond with.
🎉 This issue has been resolved in version 2.4.0 🎉 The release is available on: |
Checklist
What You Are Seeing?
Chocolatey v2.3.0 doing
upgrade
is moving the old versionlib\${PACKAGE}
tolib-bkp\${PACKAGE}
and then while attemting to download the the new package version fromhttps://community.chocolatey.org/api/v2/package/...
encounters503 (Service Unavailable: Back-end server is at capacity)
.It then fails to restore the package direcory from
lib-bkp\${PACKAGE}
tolib\${PACKAGE}
because it is missinglib\${PACKAGE]\.chocolateyPending
resulting in an uninstalled package (from chocolateys view), leaving a stalled version of the software in Windows that is not touched by any susequentupgrade
s.HTTP 503 occures multiple time a month.
Multiple clients over a long range of time are affected.
What is Expected?
On a failure to install the new package version, the old version should be restored.
How Did You Get This To Happen?
audacity
is only the example used in the corresponding log. This happens to multiple packages:choco.exe upgrade audacity --confirm --no-progress --use-package-exit-codes --log-file='C:\Windows\TEMP\3xf5qe1n.xaq\chocolatey.log'
System Details
Installed Packages
Output Log
Additional Context
No response
The text was updated successfully, but these errors were encountered: