Enhance shutdown logs with extra details#1821
Merged
Aniruddh25 merged 5 commits intomainfrom Nov 6, 2023
Merged
Conversation
…applicationshutdown trigger to stop app. Maintain reduction in dupe log events.
seantleonard
requested review from
Aniruddh25,
JelteF,
Mathos1432,
aaronburtle,
aaronpowell,
abhishekkumams,
ayush3797,
gledis69,
mbhaskar,
milismsft,
ravishetye,
severussundar and
tarazou9
as code owners
Collaborator
Did you mean to say pre 0.9 in the PR description above? |
Aniruddh25
approved these changes
Oct 18, 2023
Collaborator
Aniruddh25
left a comment
Aniruddh25
left a comment
There was a problem hiding this comment.
LGTM, thanks for the quick fix!
Contributor
|
@seantleonard , does this also address this issue: #1777 |
Contributor
Author
No, that issue calls out specific behavior of the CLI. This affects how the engine outputs errors and doesn't change CLI return codes. Engine also will return correctly a non-zero integer if startup fails. |
Aniruddh25
added a commit
that referenced
this pull request
Nov 13, 2023
## Why make this change? - Closes #1763 ## What is this change? - [Startup.cs] Adds an explicit `StopApplication()` call (https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.hosting.ihostapplicationlifetime.stopapplication?view=dotnet-plat-ext-6.0#microsoft-extensions-hosting-ihostapplicationlifetime-stopapplication) which signals the app to stop, instead of raising an Application Exception. The exception results in duplication logging of events. - [Startup.cs] Combines the Startup/DAB engine failure logging message to explain which log file caused the shutdown. - [Program.cs] Explicitly catches `TaskCanceledException` which is raised when `StopApplication()` called. In that catch block, the console error states that the engine is unable to launch. The reasons will be emitted in previous log events/exceptions. - [Program.cs] Adds back a pre 0.9 log message to the console which includes the full exception. This captures the Kestrel issue expected in the GitHub issue raised for which this PR addresses. ## How was this tested? - [x] existing test validate proper log messages are emitted. ## Sample Request(s) - add an intentional error to the runtime config to cause config parsing to fail: set global rest path to "/ap/i" ```json "rest": { "enabled": true, "path": "/ap/i", "request-body-strict": false }, ``` Observe that failure looks like:  - Start dab in a separate console window with command: ```shell dab start -c "<config file path>" ``` and see failure that elaborates on Kestrel server port unable to bind:  Co-authored-by: Aniruddh Munde <[email protected]>
Aniruddh25
added a commit
that referenced
this pull request
Nov 13, 2023
## Why make this change? This is simply cherry-picking the following PRs before creating the next stable 0.9 version: - #1876 - #1821 - #1854 - #1851 - #1872 - #1868 --------- Co-authored-by: Sean Leonard <[email protected]> Co-authored-by: Abhishek Kumar <[email protected]> Co-authored-by: neeraj-sharma2592 <[email protected]> Co-authored-by: Neeraj Sharma (from Dev Box) <[email protected]> Co-authored-by: aaronburtle <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
What is this change?
StopApplication()call (https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.hosting.ihostapplicationlifetime.stopapplication?view=dotnet-plat-ext-6.0#microsoft-extensions-hosting-ihostapplicationlifetime-stopapplication) which signals the app to stop, instead of raising an Application Exception. The exception results in duplication logging of events.TaskCanceledExceptionwhich is raised whenStopApplication()called. In that catch block, the console error states that the engine is unable to launch. The reasons will be emitted in previous log events/exceptions.How was this tested?
Sample Request(s)
Observe that failure looks like:
dab start -c "<config file path>"and see failure that elaborates on Kestrel server port unable to bind:
