Issues with APIServiceClient.shared and a custom ATRequestExecutor

[mfreiwald]

Goal

I'd like to use a custom implementation of ATRequestExecutor to track rate limits provided by the Bluesky API via the response headers.

Problem

However, I've noticed that my custom ATRequestExecutor gets overwritten whenever a new ATProtoKit instance is created. For example, when using SessionConfiguration.authenticate(with:password), the responseProvider argument is forwarded to APIClientService.shared.configure(...), which means a single shared instance of ATRequestExecutor is used across all ATProtoKit instances. Each time a new instance is created, the executor is replaced.

Why This Is an Issue

In my use case, I need to maintain two separate ATProtoKit instances: one unauthenticated and one authenticated. My custom RequestExecutor needs access to the user's access token, which I can retrieve from the SessionConfiguration’s KeychainProtocol. However, since the executor is shared (and overwritten), I can't reliably associate different executors (and their configurations) with specific ATProtoKit instances.

This issue occurs not just for my use case—there are other SDK methods and components, such as ATFacetParser.parseFacets and various view refresh methods, that internally create new ATProtoKit instances. These calls also overwrite the RequestExecutor in the shared APIClientService.

Questions

• Is there a specific reason APIClientService is implemented as a singleton shared instance?
• Are there technical advantages to sharing a single URLSession (which, as implemented now, is also replaced on each new ATProtoKit instance)?
• Would it be possible or advisable to have the API client and executor associated with each ATProtoKit instance instead?

I really appreciate the design that allows custom RequestExecutor implementations, as this makes the networking layer easy to test and mock. However, the current shared instance approach seems to limit the intended modularity and flexibility.

Thank you for your work on ATProtoKit, and I'm looking forward to any insights or suggestions you might have!

[MasterJ93]

Thanks for creating a discussion about this and I appreciate you addressing your concerns and pain points.

I’ll address your questions directly.

Is there a specific reason APIClientService is implemented as a singleton shared instance?

The reason for APIClientService having a singleton shared instance is because I wanted to have a low memory footprint with respect to creating and receiving requests. I don’t want developers to worry about ATProtoKit being a memory and CPU hog with doing even the simplest of tasks.

While this project works for Apple platforms, it’s also designed to work with Linux and Windows as well. This is especially true for Linux, as I want to ensure that a wide variety of hardware can run the project without using a lot of resources, from large servers to tiny devices like the Raspberry Pi.

I feared having multiple instances of an actor would cause the memory footprint to be high enough where it would be a problem for memory and CPU-constrained devices like the Raspberry Pi. These devices would have difficulty running it on top of the other applications and projects the developer may run on.

I also want to be a good Swift package citizen and not decide on trying to use a ton of CPU and memory just because Apple devices have a ton of them. This is not fair to the developer, and it’s not fair to your users.

Even though actors may use negligible amounts of RAM, I still don’t want to cause an issue to anyone, regardless of the hardware specifications.

Are there technical advantages to sharing a single URLSession (which, as implemented now, is also replaced on each new ATProtoKit instance)?

To directly answer your question, technically, there isn’t a benefit here.

For more context: I had the idea of sharing a single URLSession instance in mind because some developers may have some custom configurations, and I didn’t want them to have to repeatedly add the configurations over and over again.

With that said, the way it’s implemented does cause an issue where each ATProtoKit instance would override the settings was a major oversight on my part, and I’m unsure of why I didn’t consider this.

Would it be possible or advisable to have the API client and executor associated with each ATProtoKit instance instead?

This is something that I’ve thought about for a while, as APIClientService will soon be replaced with Apple’s OpenAPI generator. And given that I’ve accepted the realities of that package, it seems that it’s probably fair to implement it the way you’ve suggested, as whatever large memory and CPU footprint it would temporarily cause should be lowered when that actor is replaced. Once that happens, there will be a more streamlined and strict way of how things will work, given that the generator itself will dictate a lot of what’s going on behind the scenes.

---

We can discuss how this will be laid out with respect to what needs to happen, but from what I understand, the only think that should be done is to give each instance of ATProtoKit its own instance of APIClientService. However, please clarify your position if I’m off the mark on this.

[mfreiwald]

Thanks for you reply. I see your points.

Maybe passing in the executor on the performRequest could be also possible.
but would also mean to touch all the ATProtoKit and Bluesky methods

[MasterJ93]

Yeah, that's fine if I have to touch on all of the lexicon methods. I've gone through that before and I kind of like tedious work sometimes, so that shouldn't be a problem.

[mfreiwald]

One point I forgot to ask here:

why is the authentication header not added for custom RequestExecutors?

[mfreiwald]

No, no problems, I can add the header in the RequestExecutor, but overall it’s not clear why only this header is excluded.

for me as a developer I thought the RequestExecutor is something where I can customize the created request, so I assume there is no exceptions how the request is build.

[MasterJ93]

To clarify, is it that you were expecting it to override all of the headers of the URLRequest?

[mfreiwald]

No, the performRequest of the ATRequestExecutor should just receive the request how it was build.
and the implementation of ATRequestExecutor can override it

[MasterJ93]

Okay: I think I'm fully able to process what you were saying. Unless I'm mistaken, you're saying that that line of code is preventing custom instances of ATRequestExecutor to get the entire URLRequest. Other than that, you don't actually have any issues with how things are implemented.

If this is the case, then I can now see why that line of code can be a cause for concern, so in that case, I will remove it.

My apologies for taking this long to understand what you're coming from. I can see my line of questioning and confusion to be a point of frustration.

[mfreiwald]

Yes thanks ☺️

[mfreiwald]

Since APIServiceClient is more or less stateless (when you initialize all properties in the init) it does not need to be an actor.
I just changed it to a struct, initialize it in ATProtoKit and that's it.

Here are my changes so far: commit APIServiceClient

All the parameters of APIServiceClient.shared.config() and setLogger would need to be passed through the ATProtoKit initializer.

[MasterJ93]

As long as it's lightweight and concurrency safe, I'm fine with changing it to a struct. We can do that.

[mfreiwald]

Great I will create a Pr out of it

[mfreiwald]

Here is the PR
#173

[mfreiwald]

One important point I want to highlight—yet is completely overlooked in the new approach—is the handling of URLSession.
Each new instance of APIClientService creates its own URLSession, which is sometimes necessary, as the URLSessionConfiguration can vary depending on how the ATProtoKit instance is initialized.

However, if a developer wants to use a shared URLSession, this is also possible by leveraging the ATRequestExecutor