From 18f0a3d346c058e596a0f6c77f16524edbfdf46a Mon Sep 17 00:00:00 2001 From: "Salamon, Daniel (DF MC MTS ISW R&D DEV)" Date: Tue, 27 Nov 2018 07:23:49 +0100 Subject: [PATCH 1/5] Add missing CTS dispose to request Missing CTS dispose memory leaking. Store it in a variable and dispose it in finally --- src/Flurl.Http/FlurlRequest.cs | 51 ++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/Flurl.Http/FlurlRequest.cs b/src/Flurl.Http/FlurlRequest.cs index 0369345..74b3d7e 100644 --- a/src/Flurl.Http/FlurlRequest.cs +++ b/src/Flurl.Http/FlurlRequest.cs @@ -25,22 +25,22 @@ namespace Flurl.Http /// Url Url { get; set; } - /// - /// Creates and asynchronously sends an HttpRequestMethod. - /// Mainly used to implement higher-level extension methods (GetJsonAsync, etc). - /// - /// The HTTP method used to make the request. - /// Contents of the request body. - /// The token to monitor for cancellation requests. - /// The HttpCompletionOption used in the request. Optional. - /// A Task whose result is the received HttpResponseMessage. - Task SendAsync(HttpMethod verb, HttpContent content = null, CancellationToken cancellationToken = default(CancellationToken), HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead); - } + /// + /// Creates and asynchronously sends an HttpRequestMethod. + /// Mainly used to implement higher-level extension methods (GetJsonAsync, etc). + /// + /// The HTTP method used to make the request. + /// Contents of the request body. + /// The token to monitor for cancellation requests. + /// The HttpCompletionOption used in the request. Optional. + /// A Task whose result is the received HttpResponseMessage. + Task SendAsync(HttpMethod verb, HttpContent content = null, CancellationToken cancellationToken = default(CancellationToken), HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead); + } - /// - /// A chainable wrapper around HttpClient and Flurl.Url. - /// - public class FlurlRequest : IFlurlRequest + /// + /// A chainable wrapper around HttpClient and Flurl.Url. + /// + public class FlurlRequest : IFlurlRequest { private FlurlHttpSettings _settings; private IFlurlClient _client; @@ -116,12 +116,13 @@ namespace Flurl.Http await HandleEventAsync(Settings.BeforeCall, Settings.BeforeCallAsync, call).ConfigureAwait(false); request.RequestUri = Url.ToUri(); // in case it was modifed in the handler above - var cancellationTokenWithTimeout = cancellationToken; + var cancellationTokenWithTimeout = cancellationToken; + CancellationTokenSource timeoutTokenSource = null; - if (Settings.Timeout.HasValue) { - var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(Settings.Timeout.Value); - cancellationTokenWithTimeout = cts.Token; + if (Settings.Timeout.HasValue) { + timeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutTokenSource.CancelAfter(Settings.Timeout.Value); + cancellationTokenWithTimeout = timeoutTokenSource.Token; } call.StartedUtc = DateTime.UtcNow; @@ -133,8 +134,8 @@ namespace Flurl.Http if (Settings.CookiesEnabled) WriteRequestCookies(request); - call.Response = await Client.HttpClient.SendAsync(request, completionOption, cancellationTokenWithTimeout).ConfigureAwait(false); - call.Response.RequestMessage = request; + call.Response = await Client.HttpClient.SendAsync(request, completionOption, cancellationTokenWithTimeout).ConfigureAwait(false); + call.Response.RequestMessage = request; if (call.Succeeded) return call.Response; @@ -146,10 +147,12 @@ namespace Flurl.Http } finally { request.Dispose(); - if (Settings.CookiesEnabled) + timeoutTokenSource?.Dispose(); + + if (Settings.CookiesEnabled) ReadResponseCookies(call.Response); - call.EndedUtc = DateTime.UtcNow; + call.EndedUtc = DateTime.UtcNow; await HandleEventAsync(Settings.AfterCall, Settings.AfterCallAsync, call).ConfigureAwait(false); } } From e0af3289a36b81e9a9016c61db0b58946e1aefa9 Mon Sep 17 00:00:00 2001 From: "Salamon, Daniel (DF MC MTS ISW R&D DEV)" Date: Tue, 27 Nov 2018 07:49:02 +0100 Subject: [PATCH 2/5] Review findings TABS --- src/Flurl.Http/FlurlRequest.cs | 52 +++++++++++++++++----------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Flurl.Http/FlurlRequest.cs b/src/Flurl.Http/FlurlRequest.cs index 74b3d7e..7b021cf 100644 --- a/src/Flurl.Http/FlurlRequest.cs +++ b/src/Flurl.Http/FlurlRequest.cs @@ -25,22 +25,22 @@ namespace Flurl.Http /// Url Url { get; set; } - /// - /// Creates and asynchronously sends an HttpRequestMethod. - /// Mainly used to implement higher-level extension methods (GetJsonAsync, etc). - /// - /// The HTTP method used to make the request. - /// Contents of the request body. - /// The token to monitor for cancellation requests. - /// The HttpCompletionOption used in the request. Optional. - /// A Task whose result is the received HttpResponseMessage. - Task SendAsync(HttpMethod verb, HttpContent content = null, CancellationToken cancellationToken = default(CancellationToken), HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead); - } + /// + /// Creates and asynchronously sends an HttpRequestMethod. + /// Mainly used to implement higher-level extension methods (GetJsonAsync, etc). + /// + /// The HTTP method used to make the request. + /// Contents of the request body. + /// The token to monitor for cancellation requests. + /// The HttpCompletionOption used in the request. Optional. + /// A Task whose result is the received HttpResponseMessage. + Task SendAsync(HttpMethod verb, HttpContent content = null, CancellationToken cancellationToken = default(CancellationToken), HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead); + } - /// - /// A chainable wrapper around HttpClient and Flurl.Url. - /// - public class FlurlRequest : IFlurlRequest + /// + /// A chainable wrapper around HttpClient and Flurl.Url. + /// + public class FlurlRequest : IFlurlRequest { private FlurlHttpSettings _settings; private IFlurlClient _client; @@ -116,13 +116,13 @@ namespace Flurl.Http await HandleEventAsync(Settings.BeforeCall, Settings.BeforeCallAsync, call).ConfigureAwait(false); request.RequestUri = Url.ToUri(); // in case it was modifed in the handler above - var cancellationTokenWithTimeout = cancellationToken; - CancellationTokenSource timeoutTokenSource = null; + var cancellationTokenWithTimeout = cancellationToken; + CancellationTokenSource timeoutTokenSource = null; - if (Settings.Timeout.HasValue) { - timeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeoutTokenSource.CancelAfter(Settings.Timeout.Value); - cancellationTokenWithTimeout = timeoutTokenSource.Token; + if (Settings.Timeout.HasValue) { + timeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutTokenSource.CancelAfter(Settings.Timeout.Value); + cancellationTokenWithTimeout = timeoutTokenSource.Token; } call.StartedUtc = DateTime.UtcNow; @@ -134,8 +134,8 @@ namespace Flurl.Http if (Settings.CookiesEnabled) WriteRequestCookies(request); - call.Response = await Client.HttpClient.SendAsync(request, completionOption, cancellationTokenWithTimeout).ConfigureAwait(false); - call.Response.RequestMessage = request; + call.Response = await Client.HttpClient.SendAsync(request, completionOption, cancellationTokenWithTimeout).ConfigureAwait(false); + call.Response.RequestMessage = request; if (call.Succeeded) return call.Response; @@ -147,12 +147,12 @@ namespace Flurl.Http } finally { request.Dispose(); - timeoutTokenSource?.Dispose(); + timeoutTokenSource?.Dispose(); - if (Settings.CookiesEnabled) + if (Settings.CookiesEnabled) ReadResponseCookies(call.Response); - call.EndedUtc = DateTime.UtcNow; + call.EndedUtc = DateTime.UtcNow; await HandleEventAsync(Settings.AfterCall, Settings.AfterCallAsync, call).ConfigureAwait(false); } } From 17adfe8715760c5b3fc35428412abbdad9351844 Mon Sep 17 00:00:00 2001 From: Todd Menier Date: Fri, 11 Jan 2019 08:28:21 -0600 Subject: [PATCH 3/5] Update issue templates --- .github/ISSUE_TEMPLATE/bug_report.md | 10 ++++++++++ .github/ISSUE_TEMPLATE/feature_request.md | 10 ++++++++++ .github/ISSUE_TEMPLATE/general-questions.md | 10 ++++++++++ 3 files changed, 30 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md create mode 100644 .github/ISSUE_TEMPLATE/general-questions.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000..d4be797 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,10 @@ +--- +name: Bug report +about: Report an error or (likely) unintended behavior in the library +title: '' +labels: bug +assignees: '' + +--- + +Please provide a complete description of the bug and a code sample (or better yet, a unit test) that reproduces it. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000..6661c88 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,10 @@ +--- +name: Feature request +about: Suggest an idea for a new feature +title: '' +labels: enhancement +assignees: '' + +--- + + diff --git a/.github/ISSUE_TEMPLATE/general-questions.md b/.github/ISSUE_TEMPLATE/general-questions.md new file mode 100644 index 0000000..6faf977 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/general-questions.md @@ -0,0 +1,10 @@ +--- +name: General questions +about: Ask a general, non-programming question about the library +title: '' +labels: '' +assignees: '' + +--- + +If you have a specific programming question, please [ask on Stack Overflow](https://stackoverflow.com/questions/ask?tags=flurl). It will be answered more quickly and reach a broader audience. Thanks! From 2d176f85b88e3bc76ebf1527726397de6d3cbb33 Mon Sep 17 00:00:00 2001 From: Todd Menier Date: Fri, 11 Jan 2019 08:31:06 -0600 Subject: [PATCH 4/5] Update issue templates md link in a template wasn't the way to go --- .github/ISSUE_TEMPLATE/general-questions.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/general-questions.md b/.github/ISSUE_TEMPLATE/general-questions.md index 6faf977..00a5627 100644 --- a/.github/ISSUE_TEMPLATE/general-questions.md +++ b/.github/ISSUE_TEMPLATE/general-questions.md @@ -7,4 +7,6 @@ assignees: '' --- -If you have a specific programming question, please [ask on Stack Overflow](https://stackoverflow.com/questions/ask?tags=flurl). It will be answered more quickly and reach a broader audience. Thanks! +If you have a specific programming question, please ask on Stack Overflow. It will be answered more quickly and reach a broader audience. Thanks! + +https://stackoverflow.com/questions/ask?tags=flurl From 44a4c586de3ea04c924e30a52807491fc3cd4070 Mon Sep 17 00:00:00 2001 From: Todd Menier Date: Fri, 11 Jan 2019 08:33:33 -0600 Subject: [PATCH 5/5] Update issue templates --- .github/ISSUE_TEMPLATE/feature_request.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 6661c88..54a5e0f 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -1,10 +1,10 @@ --- name: Feature request -about: Suggest an idea for a new feature +about: Suggest an idea for a new feature or enhancement title: '' labels: enhancement assignees: '' --- - +Please describe the feature/enhancement in as much detail as possible.