context.Context support is missing

[brenol]

Issue Summary

Not really a bug, but a missing functionality. Expected to see methods with context.Context support. However, currently there is no support for it and this makes it not being possible to do proper cancellation with twilio's API.

Code Snippet

N/A

Exception/Log

N/A

Technical details:

• twilio-go version: v0.12.0
• go version: 1.16.5

[thinkingserious]

Hello @brenol,

Thank you for taking the time to report this feature request! 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.

With best regards,

Elmer

[kevinburke]

You might want to also add a note in the README that the API is going to change, unless you are going to double up every single endpoint like AWS does (CreateMessage and CreateMessageWithContext, etc.)

[natebrennand]

Hi @thinkingserious , I'd be willing to take this on if you can direct me on how you want the API's evolved. I really appreciate the ability to provide your own HTTP client, but not having accessing the context.Context is limiting our ability to instrument the client.

I envision that both the client's and API packages are going to need meaningful changes to accommodate this:

Approach 1 - break API's:

• Update client.RequestHandler & client.BaseClient to take in context.Context as the first argument.
• Then update all API's to also take in context.Context as the first argument.

Approach 2 - deprecate and expose API's w/ context.

• Add client.RequestHandlerWithCtx and client.BaseClientWithCtx
• Add API's to also take in context.Context as the first argument. otherwise same API's with WithCtx appended to the function names
• Update internals so that the DoFoo API's all call DoFooWithCtx(context.Background(), ....)
• internal methods to wrap client.RequestHandler/client.BaseClient into their context-using versions

I'm generally assuming y'all would prefer adding methods rather than breaking. Please let me know how you'd like these changes to look.

[rakatyal]

Hi @natebrennand, thank you for your suggestions! We like the approach 2, if you could submit a PR with a proof of concept, we will work on getting the changes inside the generator.

[natebrennand]

Hi @natebrennand, thank you for your suggestions! We like the approach 2, if you could submit a PR with a proof of concept, we will work on getting the changes inside the generator.

I've got PR's open to update the underlying client and the SDK to support this:

• https://github.com/twilio/twilio-go/pull/197
• https://github.com/twilio/twilio-oai-generator/pull/301

[kbolino]

Am I missing something? This is marked as done but version 1.7.1 of the API, released 5 days ago, does not seem to have support for context.

For example, the signature of CreateVerification looks like this:

func (c *ApiService) CreateVerification(ServiceSid string, params *CreateVerificationParams) (*VerifyV2Verification, error)

whereas I'm expecting it to look like this:

func (c *ApiService) CreateVerification(ctx context.Context, ServiceSid string, params *CreateVerificationParams) (*VerifyV2Verification, error)

[Makpoc]

Hello. I'd like to repeat the last question from kbolino. Support for context was added and then reverted with e89399c and 79f196e . Was this intentional change or was it a bad merge/regenerate?

[Kcrong]

Hi. I'd like to inquire about any updates regarding this issue.

[mwajeeh]

Any update here?