does not work in mult-threaded, multi-apiKey environments

[technology-shipcaddie-com]

The current implementation of easypost-csharp uses a ClientManager that is a static class, and thus is a Singleton and not thread safe because it stores the apiKey. Currently, the apiKey gets stored in the ClientManager static class and is then pulled whenever an execute happens. This works great in a single apiKey environment for both single and multi threaded environments.

This methodology breaks down quickly when in a multi-apiKey environment that is multi-threaded. The following steps show an example of the problem that happens on busy systems. Both examples below were happening frequently every day on our system when under load:

Example 1:

Thread 1:

• Client 1 sets the Client Manager api key --- EasyPost.ClientManager.SetCurrent("apiKey 1"); NOTE: singleton client Manager is now set to "apiKey 1" for everyone
• Client 1 creates shipment object and populates it (not shown) --- EasyPost.Shipment shipment = new EasyPost.Shipment();
• Client 1 gets lowest rate --- var rate = shipment.LowestRate(carriers, services);

Thread 2:

• Client 2 set the Client Manager api key --- EasyPost.ClientManager.SetCurrent("apiKey 2"); NOTE: singleton client Manager is now set to "apiKey 2" for everyone
• Client 2 creates shipment object and populates it (not shown) --- EasyPost.Shipment shipment = new EasyPost.Shipment();

Thread 1:

• Client 1 Buys label --- shipment.Buy(rate); NOTE: this function then throws an error with the following description. "Object reference not set to an instance of an object" This is because Client 1 is trying to purchase their own shipment with the "apiKey 2" of Client 2 which is in the ClientManager singleton object that the Buy method will pull to make its call.

Thread 2:

• Client 2 goes on and is able to work as if nothing has happened because the apiKey is their own "apiKey 2" value.

Example 2:

Thread 1:

• Client 1 sets the Client Manager api key --- EasyPost.ClientManager.SetCurrent("apiKey 1"); NOTE: singleton client Manager is now set to "apiKey 1" for everyone
• Client 1 creates shipment object and populates it (not shown) --- EasyPost.Shipment shipment = new EasyPost.Shipment();

Thread 2:

• Client 2 set the Client Manager api key --- EasyPost.ClientManager.SetCurrent("apiKey 2"); NOTE: singleton client Manager is now set to "apiKey 2" for everyone
• Client 2 creates shipment object and populates it (not shown) --- EasyPost.Shipment shipment = new EasyPost.Shipment();

Thread 1:

• Client 1 gets lowest rate --- var rate = shipment.LowestRate(carriers, services);
• Client 1 Buys label --- shipment.Buy(rate);
• Client 1 Gets Label from url in shipment object. NOTE: This is the worst situation of all. Client 1 purchases and gets a label using Client 2's apiKey. No error message is ever returned or indicated. All charges will show up on Client 2's account. Client 1 will not be able to look up the shipment information with the shp_..... key because it is not part of their account. Client 2 will have an extra shipment appear on their account that they cannot account for.

[technology-shipcaddie-com]

Pull request #99 addresses this issue.

[att14]

I believe #101 is a much less invasive change. If you still need it I will merge it in.

[yoelhalb]

#101 Is a very limited solution, only working when someone manually starts up threads (and not tasks) and manages them on his own (not via a ThreadPool etc.), and of course it will not help in a Web Server environment.

Of course this issue needs a solution, not just because everyone that uses it in a server environment needs it fixed, but also because it might take time till someone that uses this library in a server environment realizes the issues with the current implementation (and possibly rolls his own fix), which might mean a huge financial disaster for a company as clients might end up using the wrong apiKeys in the meantime.

[chadwixk]

Is this still an open issue? We were planning to use this project for a multi-tenant Saas application, so the repercussions of mixed client data and charges is catastrophic.

Does this fix it?

If you are operating with multiple EasyPost API keys, or wish to delegate the construction of the client requests, configure the ClientManager with a delegate at application initialization.

using EasyPost;

ClientManager.SetCurrent(() => new Client(new ClientConfiguration("yourApiKeyHere")));

And why does it state to:

configure the ClientManager with a delegate at application initialization."?

Wouldn't you set this for each request, not at App Initialization?

[att14]

Wouldn't you set this for each request, not at App Initialization?

The delegate that you configure can be a function that uses context of the request, e.g. looking up the API key in a database based on the requesting user's id.

This code is not thread-safe or asynchronous. If you are building a threaded or asynchronous application I highly suggest using https://github.com/kendallb/easypost-async-csharp.

[chadwixk]

@att14 ok, thanks for clarifying....I was looking at their project already as we wanted async support as well. Thanks for the quick response!

[TGKN]

Hi, this issue is still not fixed... #101 does not seem to be a feasible option when running the code on a webserver and #99 obviously never made it to the master branch. Are there any plans to address this issue? We are using easypost on a webserver with 250 users by now but already 5% of all calls fail because supposably the API key was "reset" between two subsequent API calls.

[att14]

@TGKN there are no plans on updating this client to handle async. My suggestion is to use https://github.com/kendallb/easypost-async-csharp instead.

[TGKN]

I am not asking to make the client being able to handle async, I am only asking for it to be thread safe. These are 2 different things. I saw the library you mentioned, It seems outdated. Easypost did some changes to their API recently, thesse are not reflected in the library you mentioned thus it's worthless putting it in a production system.

[nwithan8]

Hi, this issue is still not fixed... #101 does not seem to be a feasible option when running the code on a webserver and #99 obviously never made it to the master branch. Are there any plans to address this issue? We are using easypost on a webserver with 250 users by now but already 5% of all calls fail because supposably the API key was "reset" between two subsequent API calls.

First off, apologies in the lack of communication regarding this issue. We are actively exploring how to best address this, and weighing the idea of a refactor against how many breaking changes it might introduce.

Personally, I have little experience with running C# applications in a webserver environment, so hopefully you can educate me. If the EasyPost API client library were instance-based rather than static (meaning you had to initialize an instance of the API client and call all functions on that instance), would that be feasible in an ASP.NET environment, or would that introduce complications?

[TGKN]

Hi, Yes that would help. I can see 2 ways you can achieve this, unfortunately both introduce breaking changes.

1. Get rid of all static methods and let the consumer of your code work on an instance of the easypost client which is initialized once with an API key. This is the cleanest solution but introduces a lot of (breaking) changes
2. Add the API key to every method that triggers a call towards the easypost environment. This key is then passed to the Request class. You could keep all your static methods and the "instance" based approach would be moved outside the easypost client library. It would be up to the consumer to make sure to keep an "instance" of the api key for a user session. I think this code is less clean but easier to implement (also the workarround I chose for the last 1,5 years). It introduces breaking changes as well.

Your main problem is your ClientManager class. It is used from inside the Request class to create the API request towards easypost. As long as this class is static I don't see any way of making this library threadsafe - but maybe there is a solution I just did not think of.

[nwithan8]

Hi, Yes that would help. I can see 2 ways you can achieve this, unfortunately both introduce breaking changes.

1. Get rid of all static methods and let the consumer of your code work on an instance of the easypost client which is initialized once with an API key. This is the cleanest solution but introduces a lot of (breaking) changes
2. Add the API key to every method that triggers a call towards the easypost environment. This key is then passed to the Request class. You could keep all your static methods and the "instance" based approach would be moved outside the easypost client library. It would be up to the consumer to make sure to keep an "instance" of the api key for a user session. I think this code is less clean but easier to implement (also the workarround I chose for the last 1,5 years). It introduces breaking changes as well.

Your main problem is your ClientManager class. It is used from inside the Request class to create the API request towards easypost. As long as this class is static I don't see any way of making this library threadsafe - but maybe there is a solution I just did not think of.

I completely agree, thank you for your input. We are discussing the first idea, removing the static methods and shifting to an instance-based API client. We are also looking into a ThreadManager of sorts. We'll keep you updated.

[Justintime50]

We have released the v4 release candidate of this library which is thread-safe by having an isolated Client object. Each client object requires an API key and HTTP calls are made against this client ensuring that the API key is always tied to the correct requests, even across threads. We'd love your feedback on the release candidate if you are able. We plan to release the final v4 release within the next few weeks.

[nwithan8]

v4.0.0 of this library is officially released, making the library instance-based and thread-safe.