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

[FeatureRequest] Add CancellationToken pass-through to async request methods #514

Open
JosephIaquinto opened this issue Mar 30, 2020 · 5 comments
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap

Comments

@JosephIaquinto
Copy link

Issue Summary

I am currently building UiPath activities to wrap this SDK. UiPath is built on Windows Workflow Foundation. Workflow activities provide a cancellation token so that activities can be interrupted/ continued at a later time/ provide general async support. It would be nice if the async twilio activities exposed this parameter and passed it through to HttpClient so that requests can be cancelled. Is this something that is in the backlog or is being planned in the future?

Generally, .net libraries with async methods support this parameter with a default of CancellationToken.None as to not cause breaking changes in existing code. The .NET HttpClient supports this parameter, but the Twilio SDK does not provide it.

Code Snippet

In TwilioRestClient.cs

 public async Task<Response> RequestAsync(Request request, CancellationToken cancellationToken)
        {
            request.SetAuth(_username, _password);
            Response response;
            try
            {
                response = await HttpClient.MakeRequestAsync(request, cancellationToken);
            }
            catch (Exception clientException)
            {
                throw new ApiConnectionException(
                    "Connection Error: " + request.Method + request.ConstructUrl(),
                    clientException
                );
            }
            return ProcessResponse(response);
        }

In SystemNetHttpClient.cs

public override async Task<Response> MakeRequestAsync(Request request, CancellationToken cancellationToken)
        {
            var httpRequest = BuildHttpRequest(request);
            if (!Equals(request.Method, HttpMethod.Get))
            {
                httpRequest.Content = new FormUrlEncodedContent(request.PostParams);
            }

            this.LastRequest = request;
            this.LastResponse = null;

            var httpResponse = await _httpClient.SendAsync(httpRequest, cancellationToken).ConfigureAwait(false);
            var reader = new StreamReader(await httpResponse.Content.ReadAsStreamAsync().ConfigureAwait(false));
            // Create and return a new Response. Keep a reference to the last
            // response for debugging, but don't return it as it may be shared
            // among threads.
            var response = new Response(httpResponse.StatusCode, await reader.ReadToEndAsync().ConfigureAwait(false));
            this.LastResponse = response;
            return response;
        }

Each async version of the request functions would have this optional parameter added (Excluded here due to the fact there are a large number of them).

@childish-sambino
Copy link
Contributor

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

@childish-sambino childish-sambino added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap labels Apr 6, 2020
@gagandeepp
Copy link

Interested and willing to work on this

@guidospadavecchia
Copy link

Any updates on this?

@CrustyApplesniffer
Copy link

Any news ?

@3baaady07
Copy link

I'm trying to pass a cancellation token to

Twilio.Rest.Verify.V2.Service.VerificationCheckResource.CreateAsync(... params)

but, of course, CreateAsync does not accept one. Not even sure if the server will handle one anyways.

I thought of making a pull request, however here's a snippet from the Issues and Bugs in the Contributing to twilio-csharp guidlines.

If you find a bug in the source code or a mistake in the documentation, you can help us by submitting an issue. If the file in which the error exists has this header:

This code was generated by
\ / _    _  _|   _  _
 | (_)\/(_)(_|\/| |(/_  v1.0.0
      /       /

or

 * This code was generated by
 * ___ _ _ _ _ _    _ ____    ____ ____ _    ____ ____ _  _ ____ ____ ____ ___ __   __
 *  |  | | | | |    | |  | __ |  | |__| | __ | __ |___ |\ | |___ |__/ |__|  | |  | |__/
 *  |  |_|_| | |___ | |__|    |__| |  | |    |__] |___ | \| |___ |  \ |  |  | |__| |  \
 *
 * Twilio - XXXX
 * This is the public Twilio REST API.
 *
 * NOTE: This class is auto generated by OpenAPI Generator.
 * https://openapi-generator.tech
 * Do not edit the class manually.
 */ 

then it is a generated file and the change will need to be made by us, but submitting an issue will help us track it and keep you up-to-date. If the file isn't generated, you can help us out even more by submitting a Pull Request with a fix.

Methods that could accept a cancellation token are in such files, and therefore, contributers cannot help in this particular issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

6 participants