error messages

lib/cog/audit_message.ex

@@ -19,6 +19,10 @@ defmodule Cog.AuditMessage do
     do: "User #{request.sender.handle} denied access to '#{current_invocation}'"
   def render({:no_relays, bundle_name}, _request),
     do: ErrorResponse.render({:no_relays, bundle_name}) # Uses same user message for now
+  def render({:no_relay_group, bundle_name}, _request),
+    do: ErrorResponse.render({:no_relay_group, bundle_name})
+  def render({:no_enabled_relays, bundle_name}, _request),
+    do: ErrorResponse.render({:no_relay_group, bundle_name})
   def render({:disabled_bundle, %Bundle{name: name}}, _request),
     do: "The #{name} bundle is currently disabled"
   def render({:command_error, response}, _request),

lib/cog/command/command_resolver.ex

@@ -23,9 +23,13 @@ defmodule Cog.Command.CommandResolver do
         %SiteCommandAlias{pipeline: pipeline} ->
           {:pipeline, pipeline}
         :not_found ->
-          :not_found
+          {:error, :not_found}
+        {:not_enabled, bundle_name} ->
+          {:error, {:not_enabled, bundle_name}}
+        {:not_in_bundle, bundle_name} ->
+          {:error, {:not_in_bundle, bundle_name}}
         {:ambiguous, names} ->
-          {:ambiguous, names}
+          {:error, {:ambiguous, names}}
       end
     end
   end
@@ -51,13 +55,13 @@ defmodule Cog.Command.CommandResolver do
     case Map.get(enabled_bundles, bundle_name) do
       nil ->
         # No enabled version of the requested bundle was found
-        :not_found
+        {:not_enabled, bundle_name}
       version ->
         case Bundles.command_for_bundle_version(command_name, bundle_name, version) do
           nil ->
             # The enabled bundle version does not contain the
             # requested command
-            :not_found
+            {:not_in_bundle, bundle_name}
           result ->
             {:ok, rules} = Rules.rules_for_command(result.command)
             Cog.Command.Pipeline.ParserMeta.new(result.command.bundle.name,

lib/cog/command/pipeline/executor.ex

@@ -228,9 +228,9 @@ defmodule Cog.Command.Pipeline.Executor do
     #   fail_pipeline_with_error({:disabled_bundle, bundle}, state)
     #
     case assign_relay(relays, bundle_name, version) do
-      {nil, _} ->
-        fail_pipeline_with_error({:no_relays, bundle_name}, state)
-      {relay, relays} ->
+      {:error, reason} ->
+        fail_pipeline_with_error({reason, bundle_name}, state)
+      {:ok, {relay, relays}} ->
         topic = "/bot/commands/#{relay}/#{bundle_name}/#{command_name}"
         reply_to_topic = "#{state.topic}/reply"
         req = request_for_plan(current_plan, request, user, reply_to_topic, state.service_token)
@@ -671,19 +671,23 @@ defmodule Cog.Command.Pipeline.Executor do
       # No assignment so let's pick one
       nil ->
         case Relays.pick_one(bundle_name, bundle_version) do
-          # No relays available
-          nil ->
-            {nil, relays}
           # Store the selected relay in the relay cache
-          relay ->
-            {relay, Map.put(relays, bundle_name, relay)}
+          {:ok, relay} ->
+            {:ok, {relay, Map.put(relays, bundle_name, relay)}}
+          error ->
+            # Query DB to clarify error before reporting to the user
+            if Cog.Repository.Bundles.assigned_to_group?(bundle_name) do
+              error
+            else
+              {:error, :no_relay_group}
+            end
         end
       # Relay was previously assigned
       relay ->
         # Is the bundle still available on the relay? If not, remove the current assignment from the cache
         # and select a new relay
         if Relays.relay_available?(relay, bundle_name, bundle_version) do
-          {relay, relays}
+          {:ok, {relay, relays}}
         else
           relays = Map.delete(relays, bundle_name)
           assign_relay(relays, bundle_name, bundle_version)

lib/cog/error_response.ex

@@ -16,8 +16,16 @@ defmodule Cog.ErrorResponse do
     perms = Enum.map(Ast.Rule.permissions_used(rule), &("'#{&1}'")) |> Enum.join(", ")
     "Sorry, you aren't allowed to execute '#{current_invocation}' :(\n You will need at least one of the following permissions to run this command: #{perms}."
   end
-  def render({:no_relays, bundle_name}), # TODO: Add version, too?
-    do: "No Cog Relays supporting the `#{bundle_name}` bundle are currently online"
+  def render({:no_relays, bundle_name}) do # TODO: Add version, too?
+    "No Cog Relays supporting the `#{bundle_name}` bundle are currently online. " <>
+    "If you just assigned the bundle to a relay group you will need to wait for the relay refresh interval to pick up the new bundle."
+  end
+  def render({:no_relay_group, bundle_name}),
+    do: "Bundle '#{bundle_name}' is not assigned to a relay group. Assign the bundle to a relay group and try again."
+  def render({:no_enabled_relays, bundle_name}) do # TODO: Should we print the list of disabled relays?
+    "No Cog Relays supporting the `#{bundle_name}` bundle are currently available. " <>
+    "There are one or more relays serving the bundle, but they are disabled. Enable an appropriate relay and try again."
+  end
   def render({:disabled_bundle, %Bundle{name: name}}),
     do: "The #{name} bundle is currently disabled"
   def render({:timeout, full_command_name}),

lib/cog/relay/relays.ex

@@ -108,8 +108,12 @@ defmodule Cog.Relay.Relays do
     tracker = remove_relay(state.tracker, relay.id)
     {:reply, :ok, %{state | tracker: tracker}}
   end
-  def handle_call({:relays_running, bundle_name, version} , _from, state),
-    do: {:reply, Tracker.relays(state.tracker, bundle_name, version), state}
+  def handle_call({:relays_running, bundle_name, version} , _from, state) do
+    case Tracker.relays(state.tracker, bundle_name, version) do
+      {:ok, relays} -> {:reply, relays, state}
+      {:error, _} -> {:reply, [], state}
+    end
+  end
 
   def handle_info({:publish, @relays_discovery_topic, message}, state) do
     try do
@@ -202,8 +206,8 @@ defmodule Cog.Relay.Relays do
 
   defp random_relay(tracker, bundle, version) do
     case Tracker.relays(tracker, bundle, version) do
-      [] -> nil
-      relays -> Enum.random(relays)
+      {:ok, relays} -> {:ok, Enum.random(relays)}
+      error -> error
     end
   end
 

lib/cog/relay/tracker.ex

@@ -120,15 +120,19 @@ defmodule Cog.Relay.Tracker do
   end
 
   @doc """
-  Return a list of relays serving the specified bundle version. If the bundle is
-  disabled, return an empty list.
+  Return a tuple with the list of relays serving the bundle or an error and
+  a reason.
   """
-  @spec relays(t, bundle_name, version) :: [String.t]
+  @spec relays(t, bundle_name, version) :: {:ok, [String.t]} | {:error, atom()}
   def relays(tracker, bundle_name, bundle_version) when is_binary(bundle_name) do
-    tracker.map
-    |> Map.get({bundle_name, bundle_version}, MapSet.new)
-    |> MapSet.difference(tracker.disabled)
-    |> MapSet.to_list
+    relays = Map.get(tracker.map, {bundle_name, bundle_version}, MapSet.new)
+    enabled_relays = MapSet.difference(relays, tracker.disabled)
+
+    case {MapSet.to_list(relays), MapSet.to_list(enabled_relays)} do
+      {[], []} -> {:error, :no_relays}
+      {_, []} -> {:error, :no_enabled_relays}
+      {_, relays} -> {:ok, relays}
+    end
   end
 
   @doc """
@@ -137,8 +141,12 @@ defmodule Cog.Relay.Tracker do
   """
   @spec is_bundle_available?(t, relay_id, bundle_name, version) :: boolean()
   def is_bundle_available?(tracker, relay, bundle_name, bundle_version) do
-    available_relays = relays(tracker, bundle_name, bundle_version)
-    Enum.member?(available_relays, relay)
+    case relays(tracker, bundle_name, bundle_version) do
+      {:ok, available_relays} ->
+        Enum.member?(available_relays, relay)
+      _ ->
+        false
+    end
   end
 
   defp in_tracker?(tracker, relay_id) do

lib/cog/repository/bundles.ex

@@ -6,7 +6,7 @@ defmodule Cog.Repository.Bundles do
   require Logger
 
   alias Cog.Repo
-  alias Cog.Models.{Bundle, BundleVersion, BundleDynamicConfig, CommandVersion, Rule}
+  alias Cog.Models.{Bundle, BundleVersion, BundleDynamicConfig, CommandVersion, Rule, RelayGroupAssignment}
   alias Cog.Repository.Rules
   alias Cog.Queries
   alias Cog.BundleRegistry
@@ -26,8 +26,9 @@ defmodule Cog.Repository.Bundles do
                                                commands: [command: :bundle]]]
   @bundle_dynamic_config_preloads [:bundle]
 
+  @embedded_bundle Cog.Util.Misc.embedded_bundle
   @reserved_bundle_names [
-    Cog.Util.Misc.embedded_bundle,
+    @embedded_bundle,
     Cog.Util.Misc.site_namespace,
     "user", # a bundle named "user" would break alias resolution
     "cog"   # we're going to squat on this for now to prevent potential confusion
@@ -149,6 +150,22 @@ defmodule Cog.Repository.Bundles do
     end
   end
 
+  def assigned_to_group?(name) when name == @embedded_bundle,
+    # If it's the internal bundle we'll return true here since it doesn't
+    # run on a relay.
+    do: true
+  def assigned_to_group?(name) do
+    # If there are relay groups associated with the bundle the query will return
+    # true, otherwise it will return nil
+    query = from(b in Bundle,
+                 join: r in RelayGroupAssignment, on: b.id == r.bundle_id,
+                 where: b.name == ^name,
+                 select: true)
+
+    # Make sure that we return a boolean
+    if Repo.one(query), do: true, else: false
+  end
+
   @doc """
   Delete a bundle version, or an entire bundle (i.e., all versions for
   that bundle). You can only delete versions that are not currently

mix.lock

@@ -54,7 +54,7 @@
   "phoenix_html": {:hex, :phoenix_html, "2.7.0", "19e12e2044340c2e43df206a06d059677c59ea1868bd1c35165438d592cd420b", [:mix], [{:plug, "~> 1.0", [hex: :plug, optional: false]}]},
   "phoenix_live_reload": {:hex, :phoenix_live_reload, "1.0.5", "829218c4152ba1e9848e2bf8e161fcde6b4ec679a516259442561d21fde68d0b", [:mix], [{:fs, "~> 0.9.1", [hex: :fs, optional: false]}, {:phoenix, "~> 1.0 or ~> 1.2-rc", [hex: :phoenix, optional: false]}]},
   "phoenix_pubsub": {:hex, :phoenix_pubsub, "1.0.1", "c10ddf6237007c804bf2b8f3c4d5b99009b42eca3a0dfac04ea2d8001186056a", [:mix], []},
-  "piper": {:git, "https://github.com/operable/piper.git", "93a0034334b237cafd96cebb51257a7c3f4c14b1", []},
+  "piper": {:git, "https://github.com/operable/piper.git", "22ce7b6eecfdb51c7453b31a3c0f54d3484552c4", []},
   "plug": {:hex, :plug, "1.2.2", "cfbda521b54c92ab8ddffb173fbaabed8d8fc94bec07cd9bb58a84c1c501b0bd", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, optional: true]}, {:mime, "~> 1.0", [hex: :mime, optional: false]}]},
   "poison": {:hex, :poison, "2.2.0", "4763b69a8a77bd77d26f477d196428b741261a761257ff1cf92753a0d4d24a63", [:mix], []},
   "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], []},

test/cog/relay/tracker_test.exs

@@ -90,12 +90,17 @@ defmodule Cog.Relay.Tracker.Test do
   defp bundle_spec(name, version),
     do: {name, version}
 
-  defp assert_missing({name, version}, tracker),
-    do: assert [] = Tracker.relays(tracker, name, version)
+  defp assert_missing(bundle_spec, tracker),
+    do: assert [] = actual_relays(tracker, bundle_spec)
 
-  defp assert_relays(tracker, {name, version}, expected_relays) do
-    actual_relays = Tracker.relays(tracker, name, version)
-    assert Enum.sort(expected_relays) == Enum.sort(actual_relays)
+  defp assert_relays(tracker, bundle_spec, expected_relays),
+    do: assert Enum.sort(expected_relays) == Enum.sort(actual_relays(tracker, bundle_spec))
+
+  defp actual_relays(tracker, {name, version}) do
+    case Tracker.relays(tracker, name, version) do
+      {:ok, relays} -> relays
+      {:error, _} -> []
+    end
   end
 
 end