-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[automation/dotnet] Fix EventLogWatcher failing to read events after an exception was thrown #6821
[automation/dotnet] Fix EventLogWatcher failing to read events after an exception was thrown #6821
Conversation
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR |
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR |
@@ -91,7 +91,7 @@ private async Task ReadEventsOnce() | |||
return; | |||
} | |||
|
|||
using var fs = new FileStream(LogFile, FileMode.Open, FileAccess.Read) | |||
using var fs = new FileStream(LogFile, FileMode.Open, FileAccess.Read, FileShare.ReadWrite) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure why this fixes it at least when testing locally. Basically without this, the ExceptionPropagatesToCaller
tests would always fail on Windows 10 with the exception. Adding FileShare.ReadWrite
fixes this and the tests pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome thanks for this!
I wonder what's up with this tests not failing on CI or maybe it's not running on CI? Checking.
/run-acceptance-tests |
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR |
Please view the results of the PR Build + Acceptance Tests Run Here |
@t0yv0 The problem only occurs when event-log is used, which by default only happens during preview, which was not previously used in the error propagation test cases so I think that's why it was not happening in CI. |
Gotcha so we needed the test change to repro. Awesome. Thanks again for this. |
The problem should be reproducible by removing the |
Please view the results of the PR Build + Acceptance Tests Run Here |
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR |
…an exception was thrown (pulumi/pulumi#6821) * [automation/dotnet] Fix EventLogWatcher failing to read events after exception * Update CHANGELOG_PENDING.md Co-authored-by: Anton Tayanovskyy <anton@pulumi.com>
Discovered this while trying to preview some code which threw an exception. Instead of propagating the actual exception to the caller, the following exception was thrown
This error seems to only occur on Windows.