Skip to content

Commit

Permalink
Provided a http timeout
Browse files Browse the repository at this point in the history
The `HttpClient` does not take the `Timeout` argument which by default
is 100 seconds. Implemented a change where the user has an option to
provide the timeout. This is to address the octokit#963.
  • Loading branch information
Naveen committed Nov 24, 2015
1 parent ddcfc0a commit e2500f6
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 7 deletions.
9 changes: 9 additions & 0 deletions Octokit.Tests.Integration/Clients/GitHubClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ public async Task CanRetrieveLastApiInfoWithEtag()
}
}


[IntegrationTest]
public async Task WillFailWithHttpTimeout()
{
var github = Helper.GetAnonymousClient(TimeSpan.FromMilliseconds(5));

await Assert.ThrowsAnyAsync<System.Threading.Tasks.TaskCanceledException>(async () => await github.Repository.GetAllContributors("octokit", "octokit.net"));
}

[IntegrationTest]
public async Task CanRetrieveLastApiInfoWithLinks()
{
Expand Down
5 changes: 5 additions & 0 deletions Octokit.Tests.Integration/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public static IGitHubClient GetAuthenticatedClient()
};
}


public static GitHubClient GetAuthenticatedApplicationClient()
{
return new GitHubClient(new ProductHeaderValue("OctokitTests"))
Expand All @@ -130,6 +131,10 @@ public static IGitHubClient GetAnonymousClient()
{
return new GitHubClient(new ProductHeaderValue("OctokitTests"));
}
public static IGitHubClient GetAnonymousClient(TimeSpan httpTimeout)
{
return new GitHubClient(new ProductHeaderValue("OctokitTests"),httpTimeout);
}

public static IGitHubClient GetBadCredentialsClient()
{
Expand Down
71 changes: 71 additions & 0 deletions Octokit/GitHubClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,23 @@ public GitHubClient(ProductHeaderValue productInformation)
{
}


/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to
/// https://api.github.com/
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public GitHubClient(ProductHeaderValue productInformation,TimeSpan httpTimeout)
: this(new Connection(productInformation, GitHubApiUrl,httpTimeout))
{
}

/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to
/// https://api.github.com/
Expand All @@ -40,6 +57,22 @@ public GitHubClient(ProductHeaderValue productInformation, ICredentialStore cred
: this(new Connection(productInformation, credentialStore))
{
}
/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to
/// https://api.github.com/
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="credentialStore">Provides credentials to the client when making requests</param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public GitHubClient(ProductHeaderValue productInformation, ICredentialStore credentialStore,TimeSpan httpTimeout)
: this(new Connection(productInformation, credentialStore,httpTimeout))
{
}

/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to the specified baseAddress.
Expand All @@ -56,6 +89,25 @@ public GitHubClient(ProductHeaderValue productInformation, Uri baseAddress)
{
}


/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to the specified baseAddress.
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="baseAddress">
/// The address to point this client to. Typically used for GitHub Enterprise
/// instances</param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public GitHubClient(ProductHeaderValue productInformation, Uri baseAddress,TimeSpan httpTimeout)
: this(new Connection(productInformation, FixUpBaseUri(baseAddress),httpTimeout))
{
}

/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to the specified baseAddress.
/// </summary>
Expand All @@ -72,6 +124,25 @@ public GitHubClient(ProductHeaderValue productInformation, ICredentialStore cred
{
}

/// <summary>
/// Create a new instance of the GitHub API v3 client pointing to the specified baseAddress.
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="credentialStore">Provides credentials to the client when making requests</param>
/// <param name="baseAddress">
/// The address to point this client to. Typically used for GitHub Enterprise
/// instances</param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public GitHubClient(ProductHeaderValue productInformation, ICredentialStore credentialStore, Uri baseAddress,TimeSpan httpTimeout)
: this(new Connection(productInformation, FixUpBaseUri(baseAddress), credentialStore,httpTimeout))
{
}

/// <summary>
/// Create a new instance of the GitHub API v3 client using the specified connection.
/// </summary>
Expand Down
70 changes: 70 additions & 0 deletions Octokit/Http/Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ public Connection(ProductHeaderValue productInformation)
{
}

/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public Connection(ProductHeaderValue productInformation,TimeSpan httpTimeout)
: this(productInformation, _defaultGitHubApiUrl, _anonymousCredentials,httpTimeout)
{
}

/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
Expand Down Expand Up @@ -66,6 +81,25 @@ public Connection(ProductHeaderValue productInformation, Uri baseAddress)
{
}


/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="baseAddress">
/// The address to point this client to such as https://api.github.com or the URL to a GitHub Enterprise
/// instance</param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public Connection(ProductHeaderValue productInformation, Uri baseAddress,TimeSpan httpTimeout)
: this(productInformation, baseAddress, _anonymousCredentials,httpTimeout)
{
}

/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
Expand All @@ -79,6 +113,21 @@ public Connection(ProductHeaderValue productInformation, ICredentialStore creden
{
}

/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="credentialStore">Provides credentials to the client when making requests</param>
/// <param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
public Connection(ProductHeaderValue productInformation, ICredentialStore credentialStore,TimeSpan httpTimeout)
: this(productInformation, _defaultGitHubApiUrl, credentialStore,httpTimeout)
{
}
/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
Expand All @@ -96,6 +145,27 @@ public Connection(ProductHeaderValue productInformation, Uri baseAddress, ICrede
{
}


/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
/// <param name="productInformation">
/// The name (and optionally version) of the product using this library. This is sent to the server as part of
/// the user agent for analytics purposes.
/// </param>
/// <param name="baseAddress">
/// The address to point this client to such as https://api.github.com or the URL to a GitHub Enterprise
/// instance</param>
/// <param name="credentialStore">Provides credentials to the client when making requests</param>
///<param name="httpTimeout">
/// The number of milliseconds to wait before the request times out. The default HTTP timeout is 100000.
/// </param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
public Connection(ProductHeaderValue productInformation, Uri baseAddress, ICredentialStore credentialStore,TimeSpan httpTimeout)
: this(productInformation, baseAddress, credentialStore, new HttpClientAdapter(HttpMessageHandlerFactory.CreateDefault,httpTimeout), new SimpleJsonSerializer())
{
}

/// <summary>
/// Creates a new connection instance used to make requests of the GitHub API.
/// </summary>
Expand Down
16 changes: 13 additions & 3 deletions Octokit/Http/HttpClientAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,21 @@ public class HttpClientAdapter : IHttpClient
readonly HttpClient _http;

[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
public HttpClientAdapter(Func<HttpMessageHandler> getHandler)
public HttpClientAdapter(Func<HttpMessageHandler> getHandler)
{
Ensure.ArgumentNotNull(getHandler, "getHandler");

_http = new HttpClient(new RedirectHandler { InnerHandler = getHandler() });
}

[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
public HttpClientAdapter(Func<HttpMessageHandler> getHandler,TimeSpan httpTimeout)
{
Ensure.ArgumentNotNull(getHandler, "getHandler");
Ensure.GreaterThanZero(httpTimeout, "httpTimeout");

_http = new HttpClient(new RedirectHandler { InnerHandler = getHandler() });
_http.Timeout = httpTimeout;
}
/// <summary>
/// Sends the specified request and returns a response.
/// </summary>
Expand All @@ -37,9 +45,10 @@ public HttpClientAdapter(Func<HttpMessageHandler> getHandler)
public async Task<IResponse> Send(IRequest request, CancellationToken cancellationToken)
{
Ensure.ArgumentNotNull(request, "request");

var cancellationTokenForRequest = GetCancellationTokenForRequest(request, cancellationToken);


using (var requestMessage = BuildRequestMessage(request))
{
// Make the request
Expand All @@ -60,6 +69,7 @@ static CancellationToken GetCancellationTokenForRequest(IRequest request, Cancel
var unifiedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCancellation.Token);

cancellationTokenForRequest = unifiedCancellationToken.Token;

}
return cancellationTokenForRequest;
}
Expand Down
6 changes: 2 additions & 4 deletions SolutionInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
[assembly: AssemblyVersionAttribute("0.17.0")]
[assembly: AssemblyFileVersionAttribute("0.17.0")]
[assembly: ComVisibleAttribute(false)]
namespace System
{
internal static class AssemblyVersionInformation
{
namespace System {
internal static class AssemblyVersionInformation {
internal const string Version = "0.17.0";
}
}

0 comments on commit e2500f6

Please sign in to comment.