Add MissingDependenciesError exception

[whatyouhide]

Message is up for debate 🙃

[josevalim]

The issue comes down to the interpretation of dependency. A dependency does not mean a package. However, if we call it deps, then we are definitely tying it up to Mix interpretation of deps.

It also depends on which code we want people to write:

    If Application.spec(:foo_bar, :vsn) do

vs:

    if Code.ensure_loaded?(:foo_bar) do

At compile-time, it does not matter. At runtime, the first one is better if you assume the app may not be available. So, how do we want people to use it?

I would say this:

unless Application.spec(:foo_bar, :vsn) do
  raise MissingApplicationError,
        description: "application :foobar is required for feature xyz",
        apps: [{:foobar, "~> 1.0"}]
end

and it should raise:

** (MissingApplicationError) application :foobar is required for feature xyz
To address this, include this application as your dependency:

    {:foobar, "~> 1.0"}

So I would go with MissingApplicationError after all, especially if we have to explain why this it not a Mix dependency.

[wojtekmach]

big +1 for this. I like the MissingApplicationError name.

[wojtekmach]

Just to add some more context, I could totally see it having been used in older versions of MyXQL/Postgrex that didn't have extra_applications: [:ssl], we were raising an exception with message along the lines of, well, add extra_applications: [:ssl]! That being said perhaps apps field should be renamed to deps?

[josevalim]

We could list OTP applications and handle especially if desired. The whole issue about deps is that their are a Mix concept, it is not part of Elixir per se.

[wojtekmach]

Right, my point was not to necessarily (if that's what you're getting at) add first class support for missing OTP apps but exactly that apps: [{:foobar, "~> 1.0"}] to me kind of mixes apps and deps. I think deps: [{:foobar, "~> 1.0"}] makes more sense to me in that regard.

[whatyouhide]

The whole issue about deps is that their are a Mix concept

I disagree on this, "dependency" is a concept in software engineering not tied to Elixir or Mix, so I think it's generic enough to justify its usage here. I agree though that the field in the exception probably sounds better if the name is the same as the exception (so it would be :deps in the current PR).

[josevalim]

I disagree on this, "dependency" is a concept in software engineering not tied to Elixir or Mix

Yes but Elixir has existed for 10+ years and the meaning of the word deps to Elixir has always been clear. It is like saying "tasks" do not mean async/concurrent processing, but in Elixir they are pretty much synonyms.

[whatyouhide]

Fair enough I guess 🙃 So we can go with MissingApplicationsError + :apps field then?

[josevalim]

Given all trade offs, it seems to be the best one indeed.

[whatyouhide]

@josevalim done!

[josevalim]

Sorry, we have gone through some many bikesheds. I think last we talked it was meant to be singular? MissingApplicationError? I am fine either way, we should just double check.

[whatyouhide]

We agreed on plural IIRC, because we have a list of apps in the exception?

[josevalim]

I thought some found it weird because we don’t have many plurals in module names. Your call!

[whatyouhide]

Well it's still the singular Error so let's go with MissingApplicationsError 😄

[josevalim]

Here is my suggestion for the raising:

unless Application.spec(:foo_bar, :vsn) do
  raise MissingApplicationError,
        description: "application :foobar is required for feature xyz",
        apps: [{:foobar, "~> 1.0"}]
end

and it should raise:

** (MissingApplicationError) application :foobar is required for feature xyz
To address this, include this application as your dependency:

    {:foobar, "~> 1.0"}

I would keep them as tuples, because it is easier to copy and paste.

[whatyouhide]

I would keep them as tuples, because it is easier to copy and paste.

Good call.

[wojtekmach]

I mentioned earlier that a potential use case for this is raising on missing Elixir/OTP apps. If so, apps being non-empty makes it a bit awkward imo:

raise MissingApplicationsError,
        description: "please add :ssl to :extra_applications",
        apps: [{:ssl, ">= 0.0.0"}]

** (MissingApplicationsError) please add :ssl to :extra_applications

To address this, include these applications as your dependencies:

  {:ssl, ">= 0.0.0"}

It's OK if this exception is not intended for that. Alternatively, allow :apps to be empty?

[josevalim]

We can detect if the application comes from OTP and change the message accordingly. So no concerns here.