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

Respect start timeout setting and expose exception from server startup #117

Merged
merged 3 commits into from
Apr 5, 2018

Conversation

evanlwj
Copy link
Contributor

@evanlwj evanlwj commented Apr 4, 2018

  1. Respect the start timeout setting
  2. Expose error(exception) from server startup
  3. To be more efficient in waiting server startup

@codecov
Copy link

codecov bot commented Apr 4, 2018

Codecov Report

Merging #117 into master will decrease coverage by 0.17%.
The diff coverage is 77.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
- Coverage   66.73%   66.56%   -0.18%     
==========================================
  Files          79       79              
  Lines        2853     2862       +9     
  Branches      387      389       +2     
==========================================
+ Hits         1904     1905       +1     
- Misses        733      742       +9     
+ Partials      216      215       -1
Impacted Files Coverage Δ
src/WireMock.Net/Server/FluentMockServer.cs 50.41% <0%> (ø) ⬆️
src/WireMock.Net/Owin/OwinSelfHost.cs 89.28% <80.95%> (-10.72%) ⬇️
src/WireMock.Net/Server/FluentMockServer.Admin.cs 39.52% <0%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac72973...2ffa0af. Read the comment docs.

@StefH StefH self-requested a review April 5, 2018 06:08
{
// Throw out exception if service start fail
if (_httpServer.RunningException != null) throw new Exception($"Service start failed with error: {_httpServer.RunningException.Message}", _httpServer.RunningException);
// Repsect start timeout setting by throwing TimeoutException
Copy link
Collaborator

Choose a reason for hiding this comment

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

Respect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -29,6 +30,11 @@ interface IOwinSelfHost
/// </value>
List<int> Ports { get; }

/// <summary>
/// Determind an exception occurred in host running
Copy link
Collaborator

Choose a reason for hiding this comment

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

The exception occurred when the host is running

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks

{
server.Dispose();
IsStarted = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

No foreach (var server in servers) is needed in the finally section ???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

foreach (var server in servers) is moved into finally block. The reason of moving disposing server in finally is to make sure allocated resources(like handles in kernel mode) to be cleaned up immediately rather than wait until application is terminated.

// Dispose all servers in finally block to make sure clean up allocated resource on error happening
servers.ForEach((s) => s.Dispose());

// Throw out exception if service start fail
if (_httpServer.RunningException != null) throw new Exception($"Service start failed with error: {_httpServer.RunningException.Message}", _httpServer.RunningException);
// Repsect start timeout setting by throwing TimeoutException
if (ctsStartTimeout.IsCancellationRequested) throw new TimeoutException($"Service start timed out after {TimeSpan.FromMilliseconds(settings.StartTimeout)}");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Place throw new ... on new line with { and }.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done same

while (!_httpServer.IsStarted)
{
// Throw out exception if service start fail
if (_httpServer.RunningException != null) throw new Exception($"Service start failed with error: {_httpServer.RunningException.Message}", _httpServer.RunningException);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Place throw new ... on new line with { and }.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks

@StefH StefH merged commit 2d2a2dd into WireMock-Net:master Apr 5, 2018
StefH added a commit that referenced this pull request Apr 5, 2018
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.

2 participants