Add context to method calls

[k-capehart]

This was originally included here: 43d91e6

I decided to make this its own issue to better analyze the use cases and decide when it felt like a good candidate to be merged.

Original commit message:

This commit adds context.Context support to all Salesforce client methods and internal functions, enabling request cancellation and timeout control, and updates examples and documentation to reflect these changes.

• Inject ctx context.Context parameter into all public and internal methods and helper functions
• Switch to http.NewRequestWithContext in doRequest and propagate ctx through retry/error flows
• Update tests, examples, and README to use t.Context() or context.Background()

[jonathanmorris678]

@k-capehart This is a feature I very much need please! I'm currently working in an environment where network requests are controlled and I need to be able to pass a context to client methods (otherwise this SDK is basically unusable for me unfortunately). Is there specific information you need in order to add support for this?

Thanks for all your work on this project!

[k-capehart]

@jonathanmorris678 good to know! Do you have an example of how you would expect this to work?

Do you need to inject context for every request, or just once on initialization?

If you take a look at that commit I linked in the issue, does that look like what you need?

[derekperkins]

At a glance, that commit is what I'd expect for context integration. Definitely needed per call. Init should also take a context, but a context that isn't request scoped, and isn't canceled for the lifetime of the process, so it can do background refreshes as necessary. The library doesn't need to concern itself with that though.

[jonathanmorris678]

Agreed with @derekperkins. Per request makes sense. And yes, that commit does solve what I'm looking for—I've currently pinned my version of go-salesforce to that commit so I can use it and it seems to work well.

[k-capehart]

@derekperkins @jonathanmorris678 Only reason I'm hesitant is I want to shy away from expanding the already long argument list for a lot of these methods. Plus it'd require a new major version.

Not deal-breaking but if there's a way to get around that then I'm all ears.

[k-capehart]

Maybe we could do it similar to how the Init method already takes options. That way if you don't need it then you can just ignore, and it maintains backward compatibility.

func (sf *Salesforce) Query(query string, sObject any, options ...Option) error {
  etc
}

[jonathanmorris678]

@k-capehart Personally I don't need it to take a new context on every request, but I can see why it might be useful for others. Adding the context to Init would solve my immediate need.

Also, I realized that the authentication that happens during Init doesn't use the HTTP transport from the config (if you pass one using WithRoundTripper) and also doesn't support custom context (the original feature commit didn't add this). I currently have a fork of the repo that adds these features in the interim.

[derekperkins]

Idiomatic Go is always going to take the context as the first argument. While a single context in Init might let you add values to get through your network firewall, that doesn't allow for canceling individual calls when a request times out. This will definitely require a v4 release, it's necessary for this library to use context as Go intends.

[k-capehart]

Ah well, v4 it is then 😆
I'll find some time to dig up that commit. @jonathanmorris678 Thanks for pointing that issue out as well. If you have time, do you want to open a pull request to get your fixes into the main repo?

[jonathanmorris678]

I can probably get a PR in for the transport fix, but I likely won't have time to do the context refactor for v4! I'll stick with my fork for those changes for now.