Do not start Finch instance if an instance with same name already started?

[thojanssens]

When starting MyApp.Finch in multiple apps, I get the error:

MyApp.Finch ** (EXIT) already started

Can't we check whether the Finch instance was already started or not with that name, and if it was already, do not attempt to start it?

Problem:

A lot of libraries allow to pass an http client via the global config file. These librairies force me to have a single Finch instance across the apps.
If I have multiple apps, and try to start a Finch instance with the same name, I'll get the error above.

Solution 1: create a new app that starts a Finch instance (Shared.Finch) and shared by all other apps. But it annoys me to change the apps' design because of library limitations.

Solution 2: can Finch not just detect whether an instance with a certain name has already been started or not, and if started, ignore?

Ideally library authors allow to pass any client from different parts of the application (and so I could have AppOne.Finch, AppTwo.Finch) but while these librairies don't update and improve, it would be nice if Finch can just allow us to add Finch children in different supervisors with same name.

[wojtekmach]

I’m not sure I follow.

Do you mean that you configure deps with a finch name and it is the deps that are starting it? If so, these deps seem broken.

In any case have you tried configuring your deps with:

MyApp.FinchForDep1, MyApp.FinchForDep2, etc?

[sneako]

I agree with Wojtek. Which deps are you referring to? Ideally they should allow you to configure the finch name yourself.

[thojanssens]

@wojtekmach the deps aren't starting it.

Let me clarify through an example:

Context:
Multiple independently-deployable apps, put in an umbrella for the conveniences.

Problem:
Libraries such as sentry, swoosh, and many others suffer from a limitation where we have to pass the http client through the global config. And so they force me to have only one Finch instance:

config :swoosh, :api_client, Shared.Finch
config :sentry, client: Shared.SentryFinchHTTPClient
# etcetera

defmodule FooApp.Application do
  def start(_type, _args) do
    children = [
      FooApp.Repo,
      {Finch, name: Shared.Finch},

defmodule BarApp.Application do
  def start(_type, _args) do
    children = [
      BarApp.Repo,
      {Finch, name: Shared.Finch},

The above won't work when I start all apps in development with

mix run --no-halt

or if I execute all the tests from the umbrella root:

mix test

I'll get the error:

Shared.Finch ** (EXIT) already started

(however it will work in release/production as the apps are independently deployed and thus won't have name collision)

Solutions:

1. Create a new Umbrella app with a supervision tree, shared by the apps, and start a new Finch instance from there.

=> I'm annoyed because it seems I introduce an unnecessary complexity: a new supervision tree, a start-able dependency, and I do not even need it for release/production but only to satisfy the problem in development (i.e. running all apps and testing all apps simultaneously).

2. Ideally I have multiple Finch instances per app, so that I can also adjust their configurations independently. I am raising awareness to library authors:

• getsentry/sentry-elixir#639
• swoosh/swoosh#767 (comment)

Note that I had a private chat with Jose and he agrees those are library limitations because of anti-patterns (libraries having been developed quite some time ago before awareness of those anti-patterns was raised). Also see: https://hexdocs.pm/elixir/1.15.7/library-guidelines.html#avoid-application-configuration

=> This won't be feasible in any way until the library authors update the code. But say even if solved, in the future I might need to rely on yet another library that shares the same limitation again.

3. As asked via this issue, can Finch just start a Finch instance for a specific name only once, and when a new Finch instance is attempted to be started with same name, it is not started/ignored?

Is it possible to allow to handle the :already_started error, allowing a developer to ignore the error, so that I can be in an environment as described where:

• I can run and test multiple apps simultaneously in dev;
• I can have the apps independently deployed in prod;
• I do not need to introduce a new shared supervision tree.

Thank you for your attention, as it is a bit long.

[wojtekmach]

Ah, of course. That makes sense. Yeah, ideally we solve it in libraries. It's really great that you have started the discussions there.

One less than ideal solution that comes into mind is to do the change you want to make not in Finch but in a custom, say, Shared.Finch.child_spec/1. And then you replace:

- {Finch, name: Shared.Finch}
+ {Shared.Finch, name: Shared.Finch}

You have to put this shared module Shared.Finch somewhere though e.g. in another umbrella child app.

[thojanssens]

@wojtekmach would it look something like below? The problem is that returning nil doesn't work

returned an error: bad child specification, invalid child specification

and I don't know how to specify a no-op.

defmodule Shared.Finch do
  def child_spec(opts) do
    name = Keyword.fetch!(opts, :name)

    case Process.whereis(name) do
      nil ->
        Finch.child_spec(opts)

      _pid ->
        # no-op?
        nil
    end
  end
end

[wojtekmach]

I don’t remember, maybe returning atom :ignore does the trick. You can also return a no op Task spec: {Task, fn -> :ok end}. You can also build child spec map yourself, return a spec that calls start_link in this module. And your custom start_link calls Finch.start_link and pattern matches on it. That’s probably the best way to go.

[thojanssens]

Okay @wojtekmach ty I'll go for that solution. That allows me to share a Finch instance across multiple supervisors.

defmodule Shared.Finch do
  def child_spec(opts) do
    name = Keyword.fetch!(opts, :name)

    case Process.whereis(name) do
      nil ->
        Finch.child_spec(opts)

      _pid ->
        %{
          id: Ecto.UUID.generate(),
          start: {Task, :start_link, [fn -> :ok end]},
          restart: :temporary
        }
    end
  end
end

I feel safer to delegate the child_spec call to Finch:D I call Ecto.UUID.generate() to generate an id to avoid id collisions, not sure that's dumb or not.

So I think we can close this now?

[wojtekmach]

Yes please close :)

the :id needs to be unique per supervisor. It’s fine if it’s opts[:name].

[thojanssens]

<3<3<3<3<3<3