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

Change exceptions thrown in setup tasks into task failures, and add more details to error logging #1071

Merged
merged 5 commits into from
Jun 7, 2023

Conversation

florelis
Copy link
Member

@florelis florelis commented Jun 6, 2023

Summary of the pull request

Currently, if there is an error in the elevated install task, we would just propagate the exception, instead of logging it and marking the task and failed. And on the main process side, we assumed that any error would show up as a task failure and we would not deal with exceptions. Then, on the loading page, when an exception occurred we would not set the task result state.

This all resulted in exceptions on the elevated install task side causing the task to disappear from the summary page, as it had no valid state.

This PR adds some better guardrails in these places to handle exceptions. It also updates the logging of errors to better use the logger we have that prints the full exception details by default, instead of manually getting the exception message without much detail.

References and relevant issues

https://task.ms/44717643

Detailed description of the pull request / Additional comments

Validation steps performed

PR checklist

@florelis florelis merged commit 770f6dc into microsoft:main Jun 7, 2023
@florelis florelis deleted the exceptions branch June 7, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apps to install are dropped from the UI if there is an exception in the elevated process
3 participants