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

Switch FAIL_FAST to LOG for starting inbound connection server on monarch startup #10261

Merged
2 commits merged into from
May 28, 2021

Conversation

miniksa
Copy link
Member

@miniksa miniksa commented May 28, 2021

Stop startup crash by logging when monarch fails to register inbound connections, but still crash when COM attempted to start us

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • This should stop the crash on launch until we can get the internal teams to resolve the catalog issue
  • I left the COM -Embedding start fail fast though so it won't take forever to time out (as default timeout is 3-5 minutes). I will change that if it becomes necessary.

Validation Steps Performed

  • I basically have to guess at this one based on the crash dump and Watson logs because it happens sporadically when the platform messes up on us.

…connections, but still crash when COM attempted to start us
@ghost ghost added Area-DefApp Priority-1 A description (P1) Severity-Crash Crashes are real bad news. labels May 28, 2021
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gods when did our code get so complicated

@miniksa
Copy link
Member Author

miniksa commented May 28, 2021

gods when did our code get so complicated

You know exactly what I'm going to say here.

@DHowett
Copy link
Member

DHowett commented May 28, 2021

@msftbot merge this in 2 minutes

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 28, 2021
@ghost
Copy link

ghost commented May 28, 2021

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Fri, 28 May 2021 18:19:12 GMT, which is in 2 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@DHowett DHowett added the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label May 28, 2021
@DHowett DHowett changed the title Switch fail fast to log for starting inbound connection server on monarch startup Switch FAIL_FAST to LOG for starting inbound connection server on monarch startup May 28, 2021
@ghost ghost merged commit d8647e0 into main May 28, 2021
@ghost ghost deleted the dev/miniksa/def_crash_catalog branch May 28, 2021 20:57
DHowett pushed a commit that referenced this pull request Jun 1, 2021
…arch startup (#10261)

Stop startup crash by logging when monarch fails to register inbound connections, but still crash when COM attempted to start us

## References
- See also #10243

## PR Checklist
* [x] Closes #10233
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA

## Detailed Description of the Pull Request / Additional comments
- This should stop the crash on launch until we can get the internal teams to resolve the catalog issue
- I left the COM -Embedding start fail fast though so it won't take forever to time out (as default timeout is 3-5 minutes). I will change that if it becomes necessary.

## Validation Steps Performed
- I basically have to guess at this one based on the crash dump and Watson logs because it happens sporadically when the platform messes up on us.

(cherry picked from commit d8647e0)
@DHowett DHowett removed the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label Jul 7, 2021
@ghost
Copy link

ghost commented Jul 14, 2021

🎉Windows Terminal v1.9.1942.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Jul 14, 2021
@ghost
Copy link

ghost commented Jul 14, 2021

🎉Windows Terminal Preview v1.10.1933.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-DefApp AutoMerge Marked for automatic merge by the bot when requirements are met Priority-1 A description (P1) Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash on startup, related to defterm
3 participants