diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs index 970a082e..b5b60c5f 100644 --- a/.dialyzer_ignore.exs +++ b/.dialyzer_ignore.exs @@ -25,6 +25,14 @@ {"deps/cloak/lib/cloak/vault.ex", :unknown_function}, {"deps/cloak_ecto/lib/cloak_ecto/type.ex", :callback_info_missing}, {"deps/cloak_ecto/lib/cloak_ecto/types/binary.ex", :unknown_function}, + # Cloak.Vault uses GenServer, whose callback info isn't in the PLT because the + # containing gen_server.ex path is a CI build artifact, not a local dep path. + ~r/gen_server\.ex/, + + # :ranch is a transitive dep (via Bandit/Cowboy); :ranch.get_addr/1 exists + # but ranch isn't in the default PLT graph. Adding :ranch to plt_add_apps + # destabilises the PLT, so suppress the single call site instead. + {"lib/towerops_web/remote_ip.ex", :unknown_function}, # Vendored SnmpKit — out of scope for typing cleanup ~r/lib\/snmpkit/, diff --git a/lib/mix/tasks/oban.cancel_stuck_discovery.ex b/lib/mix/tasks/oban.cancel_stuck_discovery.ex index d91cf7ac..51264e6d 100644 --- a/lib/mix/tasks/oban.cancel_stuck_discovery.ex +++ b/lib/mix/tasks/oban.cancel_stuck_discovery.ex @@ -28,39 +28,17 @@ defmodule Mix.Tasks.Oban.CancelStuckDiscovery do |> where([j], j.state in ["scheduled", "retryable", "executing"]) |> Towerops.Repo.all() |> Enum.map(fn job -> - case Oban.cancel_job(Oban, job.id) do - {:ok, cancelled_job} -> - Logger.info( - "Cancelled stuck discovery job", - job_id: cancelled_job.id, - device_id: get_in(cancelled_job.args, ["device_id"]), - state: cancelled_job.state, - attempted_at: cancelled_job.attempted_at - ) + :ok = Oban.cancel_job(Oban, job.id) - 1 + Logger.info( + "Cancelled stuck discovery job", + job_id: job.id, + device_id: get_in(job.args, ["device_id"]), + state: job.state, + attempted_at: job.attempted_at + ) - :ok -> - # Some versions of Oban return :ok instead of {:ok, job} - Logger.info( - "Cancelled stuck discovery job", - job_id: job.id, - device_id: get_in(job.args, ["device_id"]), - state: job.state, - attempted_at: job.attempted_at - ) - - 1 - - {:error, reason} -> - Logger.error( - "Failed to cancel job", - job_id: job.id, - error: inspect(reason) - ) - - 0 - end + 1 end) |> Enum.sum() diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index 3dd4f254..8f791c1e 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -122,6 +122,9 @@ defmodule Towerops.Accounts do {:error, %Ecto.Changeset{}} """ + # Ecto.Multi uses an opaque type that dialyzer can't see through when + # composing with local helpers; suppress the false-positive opaque warning. + @dialyzer {:no_opaque, [register_user: 1]} @spec register_user(map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def register_user(attrs) do user_changeset = User.registration_changeset(%User{}, attrs) @@ -1771,6 +1774,8 @@ defmodule Towerops.Accounts do @doc """ Confirms a user by the given token. """ + # Ecto.Multi uses an opaque type that dialyzer can't always see through. + @dialyzer {:no_opaque, [confirm_user: 1]} def confirm_user(token) do with {:ok, query} <- UserToken.verify_email_token_query(token, "confirm"), %User{} = user <- Repo.one(query), diff --git a/lib/towerops/accounts/user.ex b/lib/towerops/accounts/user.ex index b86329b0..915016b5 100644 --- a/lib/towerops/accounts/user.ex +++ b/lib/towerops/accounts/user.ex @@ -44,11 +44,13 @@ defmodule Towerops.Accounts.User do timestamps(type: :utc_datetime) end + # Fields default to nil in built-but-unpersisted structs (e.g. `%User{}` used + # to seed a registration form), so the type permits nil where appropriate. @type t :: %__MODULE__{ - id: Ecto.UUID.t(), - email: String.t(), + id: Ecto.UUID.t() | nil, + email: String.t() | nil, password: String.t() | nil, - hashed_password: String.t(), + hashed_password: String.t() | nil, confirmed_at: DateTime.t() | nil, authenticated_at: DateTime.t() | nil, is_superuser: boolean(), @@ -60,8 +62,8 @@ defmodule Towerops.Accounts.User do last_sudo_at: DateTime.t() | nil, memberships: NotLoaded.t() | [Membership.t()], organizations: NotLoaded.t() | [Organization.t()], - inserted_at: DateTime.t(), - updated_at: DateTime.t() + inserted_at: DateTime.t() | nil, + updated_at: DateTime.t() | nil } @doc """ diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index cf4d5d45..3cb56af2 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -341,7 +341,9 @@ defmodule Towerops.Agents do Returns `{:ok, %{notified: N, skipped: N, version: String.t()}}` on success. """ - @spec broadcast_mass_update() :: {:ok, map()} | {:error, term()} + @spec broadcast_mass_update() :: + {:ok, %{notified: non_neg_integer(), skipped: non_neg_integer(), version: String.t()}} + | {:error, term()} def broadcast_mass_update do ReleaseChecker.invalidate_cache() diff --git a/lib/towerops/agents/stats.ex b/lib/towerops/agents/stats.ex index dccc891c..00edd4d0 100644 --- a/lib/towerops/agents/stats.ex +++ b/lib/towerops/agents/stats.ex @@ -373,7 +373,12 @@ defmodule Towerops.Agents.Stats do ] } """ - @spec get_device_assignment_with_latency(Ecto.UUID.t()) :: map() + @spec get_device_assignment_with_latency(Ecto.UUID.t()) :: %{ + device_id: Ecto.UUID.t(), + current_agent_token_id: Ecto.UUID.t() | nil, + assignment_source: :device | :site | :organization | :global | :none, + latency_stats: [map()] + } def get_device_assignment_with_latency(device_id) do device = Device diff --git a/lib/towerops/alerts/storm_detector.ex b/lib/towerops/alerts/storm_detector.ex index cd687c0c..325c3c34 100644 --- a/lib/towerops/alerts/storm_detector.ex +++ b/lib/towerops/alerts/storm_detector.ex @@ -134,8 +134,8 @@ defmodule Towerops.Alerts.StormDetector do now_ms = System.system_time(:millisecond) alert_timestamps = - state.alert_timestamps - |> :queue.in(now_ms) + now_ms + |> :queue.in(state.alert_timestamps) |> prune_old_timestamps(now_ms - 60_000) |> limit_queue_size(@max_alert_queue_size) diff --git a/lib/towerops/api_tokens.ex b/lib/towerops/api_tokens.ex index affb0594..58453f7e 100644 --- a/lib/towerops/api_tokens.ex +++ b/lib/towerops/api_tokens.ex @@ -182,7 +182,8 @@ defmodule Towerops.ApiTokens do |> Base.encode16(case: :lower) end - @spec update_last_used(ApiToken.t()) :: :ok + @spec update_last_used(ApiToken.t()) :: + {:ok, ApiToken.t() | pid()} | {:error, Ecto.Changeset.t()} defp update_last_used(token) do timestamp = Towerops.Time.now() @@ -204,7 +205,7 @@ defmodule Towerops.ApiTokens do |> Repo.update() end - @spec spawn_async_update(ApiToken.t(), DateTime.t()) :: :ok + @spec spawn_async_update(ApiToken.t(), DateTime.t()) :: {:ok, pid()} defp spawn_async_update(token, timestamp) do parent = self() diff --git a/lib/towerops/billing/stripe_client.ex b/lib/towerops/billing/stripe_client.ex index bfc67271..a8c50d01 100644 --- a/lib/towerops/billing/stripe_client.ex +++ b/lib/towerops/billing/stripe_client.ex @@ -304,13 +304,6 @@ defmodule Towerops.Billing.StripeClient do end) end - defp get_retry_after(headers) when is_list(headers) do - case List.keyfind(headers, "retry-after", 0) do - {_, value} -> String.to_integer(value) - nil -> 60 - end - end - defp get_retry_after(headers) when is_map(headers) do case Map.get(headers, "retry-after") do [value | _] -> String.to_integer(value) diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index 975535af..f0fd9403 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -19,6 +19,18 @@ defmodule Towerops.Devices do @device_preloads [:organization, site: :organization] + # Ecto.Multi uses opaque types that dialyzer can't see through when composed + # with local helper functions for locking and quota checks. + @dialyzer {:no_opaque, + [ + create_device: 1, + create_device: 2, + add_organization_lock: 2, + add_quota_check: 2, + maybe_lock_organization_for_quota: 3, + maybe_check_device_quota: 3 + ]} + @doc """ Returns the list of devices for a site. Ordered by custom display_order (if set), then alphabetically by name. diff --git a/lib/towerops/devices/mikrotik_backups/differ.ex b/lib/towerops/devices/mikrotik_backups/differ.ex index 4e6ca073..5e7cab32 100644 --- a/lib/towerops/devices/mikrotik_backups/differ.ex +++ b/lib/towerops/devices/mikrotik_backups/differ.ex @@ -122,11 +122,10 @@ defmodule Towerops.Devices.MikrotikBackups.Differ do end # Replace regex matches where capture group 1 is a prefix to keep and - # capture group 2 is the sensitive value to mask - defp mask_pattern(pattern, config, caseless) do - opts = if caseless, do: "i", else: "" - - case Regex.compile(pattern, opts) do + # capture group 2 is the sensitive value to mask. Always case-insensitive — + # all current callers pass `true`. + defp mask_pattern(pattern, config) do + case Regex.compile(pattern, "i") do {:ok, regex} -> Regex.replace(regex, config, fn _full, prefix, value -> hash = sha256_short(value) @@ -162,7 +161,7 @@ defmodule Towerops.Devices.MikrotikBackups.Differ do # Mask password=VALUE patterns (case-insensitive) defp mask_passwords(config) do - mask_pattern("(password=)(\\S+)", config, true) + mask_pattern("(password=)(\\S+)", config) end # Mask SNMP community strings via two sub-patterns @@ -196,7 +195,7 @@ defmodule Towerops.Devices.MikrotikBackups.Differ do # Mask secret=VALUE patterns (case-insensitive) defp mask_ipsec_secrets(config) do - mask_pattern("(secret=)(\\S+)", config, true) + mask_pattern("(secret=)(\\S+)", config) end # Mask wireless WPA and WPA2 pre-shared key patterns (case-insensitive) @@ -209,6 +208,6 @@ defmodule Towerops.Devices.MikrotikBackups.Differ do # Mask a single wireless key pattern: KEY_NAME=VALUE (case-insensitive) defp mask_wpa_key(config, key_name) do pattern = "(#{key_name}=)(\\S+)" - mask_pattern(pattern, config, true) + mask_pattern(pattern, config) end end diff --git a/lib/towerops/ecto_types/json_any.ex b/lib/towerops/ecto_types/json_any.ex index 7c8c028f..a61e7b03 100644 --- a/lib/towerops/ecto_types/json_any.ex +++ b/lib/towerops/ecto_types/json_any.ex @@ -30,7 +30,7 @@ defmodule Towerops.EctoTypes.JsonAny do def cast(_), do: :error @impl Ecto.Type - @spec load(term()) :: {:ok, json_value()} | :error + @spec load(term()) :: {:ok, json_value()} def load(value), do: {:ok, value} @impl Ecto.Type diff --git a/lib/towerops/gaiia/client.ex b/lib/towerops/gaiia/client.ex index e43b8b1e..a7c8d132 100644 --- a/lib/towerops/gaiia/client.ex +++ b/lib/towerops/gaiia/client.ex @@ -336,8 +336,6 @@ defmodule Towerops.Gaiia.Client do end end - defp get_retry_after(_headers), do: 60 - defp parse_retry_after(value) when is_binary(value) do case Integer.parse(value) do {seconds, _} -> seconds diff --git a/lib/towerops/gaiia/sync.ex b/lib/towerops/gaiia/sync.ex index 767e8812..7de29c77 100644 --- a/lib/towerops/gaiia/sync.ex +++ b/lib/towerops/gaiia/sync.ex @@ -203,10 +203,6 @@ defmodule Towerops.Gaiia.Sync do defp humanize_sync_error(:forbidden), do: "Access denied — your API key may not have sufficient permissions" - defp humanize_sync_error(:not_found), do: "API endpoint not found — the Gaiia API may have changed" - - defp humanize_sync_error(:timeout), do: "Connection timed out — Gaiia may be temporarily unavailable" - defp humanize_sync_error(%Req.TransportError{reason: :econnrefused}), do: "Connection refused — unable to reach Gaiia" defp humanize_sync_error(%Req.TransportError{reason: reason}) do diff --git a/lib/towerops/maintenance.ex b/lib/towerops/maintenance.ex index 07f60d71..fbdb144c 100644 --- a/lib/towerops/maintenance.ex +++ b/lib/towerops/maintenance.ex @@ -166,7 +166,7 @@ defmodule Towerops.Maintenance do Much more efficient than calling `device_in_maintenance?/1` for each device individually (e.g., during an alert storm with 500 devices). """ - @spec devices_in_maintenance(list(String.t())) :: MapSet.t() + @spec devices_in_maintenance(list(String.t())) :: map() def devices_in_maintenance(device_ids) when is_list(device_ids) do if Enum.empty?(device_ids) do MapSet.new() diff --git a/lib/towerops/netbox/sync.ex b/lib/towerops/netbox/sync.ex index 8b7981ce..817fb903 100644 --- a/lib/towerops/netbox/sync.ex +++ b/lib/towerops/netbox/sync.ex @@ -227,19 +227,8 @@ defmodule Towerops.NetBox.Sync do defp humanize_sync_error(:unauthorized), do: "Authentication failed — check your NetBox API token" - defp humanize_sync_error(:forbidden), do: "Access denied — your API token may not have sufficient permissions" - defp humanize_sync_error(:not_found), do: "API endpoint not found — check your NetBox URL" - defp humanize_sync_error(:timeout), do: "Connection timed out — NetBox may be temporarily unavailable" - - defp humanize_sync_error(%Req.TransportError{reason: :econnrefused}), do: "Connection refused — unable to reach NetBox" - - defp humanize_sync_error(%Req.TransportError{reason: reason}) do - Logger.warning("NetBox sync transport error: #{inspect(reason)}") - "Connection error — unable to establish a secure connection to NetBox" - end - defp humanize_sync_error(reason) when is_binary(reason) do if String.contains?(reason, ["CA trust store", "cacert", "certificate"]) do "SSL certificate verification failed — unable to establish a secure connection" @@ -248,11 +237,4 @@ defmodule Towerops.NetBox.Sync do "An unexpected error occurred during sync" end end - - defp humanize_sync_error({:unexpected_status, status}), do: "NetBox returned unexpected HTTP #{status}" - - defp humanize_sync_error(reason) do - Logger.warning("NetBox sync failed with unexpected error: #{inspect(reason)}") - "An unexpected error occurred during sync" - end end diff --git a/lib/towerops/organizations.ex b/lib/towerops/organizations.ex index 27f8c41f..15c2a15e 100644 --- a/lib/towerops/organizations.ex +++ b/lib/towerops/organizations.ex @@ -87,6 +87,8 @@ defmodule Towerops.Organizations do """ @dialyzer {:nowarn_function, create_organization: 2} @dialyzer {:nowarn_function, create_organization: 3} + # Ecto.Multi/Changeset opaqueness trips dialyzer on this helper too. + @dialyzer {:no_opaque, [do_create_organization: 4, set_default_organization: 2]} @spec create_organization(map(), String.t(), Keyword.t()) :: {:ok, Organization.t()} | {:error, Ecto.Changeset.t()} def create_organization(attrs, user_id, opts \\ []) do bypass_limits = Keyword.get(opts, :bypass_limits, false) diff --git a/lib/towerops/preseem/sync.ex b/lib/towerops/preseem/sync.ex index 8d62470c..cdac1c31 100644 --- a/lib/towerops/preseem/sync.ex +++ b/lib/towerops/preseem/sync.ex @@ -136,10 +136,8 @@ defmodule Towerops.Preseem.Sync do defp run_device_matching(org_id) do if Code.ensure_loaded?(DeviceMatcher) do - case DeviceMatcher.match_unmatched(org_id) do - {:ok, count} -> count - _ -> 0 - end + {:ok, count} = DeviceMatcher.match_unmatched(org_id) + count else 0 end @@ -172,8 +170,6 @@ defmodule Towerops.Preseem.Sync do defp humanize_sync_error(:unauthorized), do: "Authentication failed — check your API key" - defp humanize_sync_error(:timeout), do: "Connection timed out — Preseem may be temporarily unavailable" - defp humanize_sync_error(reason) when is_binary(reason) do if String.contains?(reason, ["CA trust store", "cacert", "certificate"]) do "SSL certificate verification failed — unable to establish a secure connection" diff --git a/lib/towerops/proto/agent.ex b/lib/towerops/proto/agent.ex index dc66e9da..e13f3430 100644 --- a/lib/towerops/proto/agent.ex +++ b/lib/towerops/proto/agent.ex @@ -618,8 +618,6 @@ defmodule Towerops.Agent.SnmpResult do defp uppercase_job_type(:test_credentials), do: :TEST_CREDENTIALS defp uppercase_job_type(:ping), do: :PING defp uppercase_job_type(:lldp_topology), do: :LLDP_TOPOLOGY - # Already uppercase - defp uppercase_job_type(other), do: other end defmodule Towerops.Agent.SnmpResult.OidValuesEntry do diff --git a/lib/towerops/proto/types.ex b/lib/towerops/proto/types.ex index 6610c8f2..b709d345 100644 --- a/lib/towerops/proto/types.ex +++ b/lib/towerops/proto/types.ex @@ -12,7 +12,7 @@ defmodule Towerops.Proto.Types do @type job_type :: :discover | :poll | :mikrotik | :test_credentials | :ping | :lldp_topology @doc "Convert JobType atom to protobuf integer" - @spec job_type_to_int(job_type()) :: non_neg_integer() + @spec job_type_to_int(job_type()) :: 0 | 1 | 2 | 3 | 4 | 5 def job_type_to_int(:discover), do: 0 def job_type_to_int(:poll), do: 1 def job_type_to_int(:mikrotik), do: 2 diff --git a/lib/towerops/proto/wire.ex b/lib/towerops/proto/wire.ex index d471f442..ee4f12c8 100644 --- a/lib/towerops/proto/wire.ex +++ b/lib/towerops/proto/wire.ex @@ -43,7 +43,8 @@ defmodule Towerops.Proto.Wire do Decode a varint from the front of a binary. Returns the value and remaining bytes. """ - @spec decode_varint(binary()) :: {:ok, {non_neg_integer(), binary()}} | {:error, wire_error()} + @spec decode_varint(binary()) :: + {:ok, {non_neg_integer(), binary()}} | {:error, :invalid_varint | :unexpected_eof} def decode_varint(data), do: decode_varint_loop(data, 0, 0) defp decode_varint_loop(<>, value, shift) do @@ -108,7 +109,8 @@ defmodule Towerops.Proto.Wire do @doc """ Decode a length-delimited field. Returns (field_bytes, rest). """ - @spec decode_bytes(binary()) :: {:ok, {binary(), binary()}} | {:error, wire_error()} + @spec decode_bytes(binary()) :: + {:ok, {binary(), binary()}} | {:error, :invalid_varint | :unexpected_eof} def decode_bytes(data) do case decode_varint(data) do {:ok, {len, rest}} -> @@ -150,7 +152,7 @@ defmodule Towerops.Proto.Wire do @doc """ Encode a signed int64 as a varint (two's complement for negatives). """ - @spec encode_int64(integer()) :: binary() + @spec encode_int64(integer()) :: nonempty_binary() def encode_int64(value) when value < 0 do # Two's complement: add 2^64 unsigned = value + 18_446_744_073_709_551_616 @@ -270,7 +272,7 @@ defmodule Towerops.Proto.Wire do Note: In protobuf, zero values are typically omitted, but in OTP 27+ pattern matching on 0.0 only matches +0.0. We encode all values to avoid this issue. """ - @spec encode_double_field(iodata(), pos_integer(), float()) :: iodata() + @spec encode_double_field(iodata(), pos_integer(), float()) :: nonempty_maybe_improper_list() def encode_double_field(builder, field_number, value) do [ builder, diff --git a/lib/towerops/snmp.ex b/lib/towerops/snmp.ex index 356f9945..3983b3f0 100644 --- a/lib/towerops/snmp.ex +++ b/lib/towerops/snmp.ex @@ -89,7 +89,8 @@ defmodule Towerops.Snmp do iex> discover_all_for_org(org_id) {:ok, %{success: 10, failed: 2, errors: [:timeout, :no_response]}} """ - @spec discover_all_for_org(String.t()) :: {:ok, map()} + @spec discover_all_for_org(String.t()) :: + {:ok, %{enqueued: non_neg_integer(), failed: non_neg_integer(), errors: [term()]}} def discover_all_for_org(org_id) do Discovery.discover_all(org_id) end diff --git a/lib/towerops/snmp/adapters/replay.ex b/lib/towerops/snmp/adapters/replay.ex index 7a37af25..075ace73 100644 --- a/lib/towerops/snmp/adapters/replay.ex +++ b/lib/towerops/snmp/adapters/replay.ex @@ -135,7 +135,7 @@ defmodule Towerops.Snmp.Adapters.Replay do # Parse string values from protobuf (agent sends everything as strings) # This is the critical type inference logic - @spec parse_value(String.t()) :: term() + @spec parse_value(String.t()) :: nil | binary() | number() | [term()] defp parse_value(value) when is_binary(value) do cond do # Empty string (from ASN_NULL values) - check FIRST diff --git a/lib/towerops/snmp/deferred_discovery.ex b/lib/towerops/snmp/deferred_discovery.ex index 516e3f95..c6d45cbb 100644 --- a/lib/towerops/snmp/deferred_discovery.ex +++ b/lib/towerops/snmp/deferred_discovery.ex @@ -75,7 +75,8 @@ defmodule Towerops.Snmp.DeferredDiscovery do Deferred checks are for non-critical data that's nice to have. If they fail or timeout, discovery succeeds with a note about what's missing. """ - @spec deferred_check(Client.connection_opts(), (-> term()), keyword()) :: check_result() + @spec deferred_check(Client.connection_opts(), (-> term()), keyword()) :: + {:ok, term()} | {:timeout, term()} | {:deferred, term(), term()} def deferred_check(_client_opts, check_fn, opts \\ []) do timeout = Keyword.get(opts, :timeout, @default_deferred_timeout) default_value = Keyword.get(opts, :default, []) diff --git a/lib/towerops/snmp/discovery.ex b/lib/towerops/snmp/discovery.ex index 14dabd3c..1af7484b 100644 --- a/lib/towerops/snmp/discovery.ex +++ b/lib/towerops/snmp/discovery.ex @@ -745,7 +745,14 @@ defmodule Towerops.Snmp.Discovery do end @spec build_device_info(Client.connection_opts(), system_info(), profile()) :: - {:ok, device_info()} + {:ok, + %{ + required(:manufacturer) => String.t(), + required(:model) => String.t(), + optional(:firmware_version) => String.t() | nil, + optional(:serial_number) => String.t() | nil, + optional(atom()) => term() + }} defp build_device_info(client_opts, system_info, {:yaml, profile}) do # Use YAML profile to identify device identified_info = Dynamic.identify_device(profile, client_opts, system_info) @@ -853,7 +860,10 @@ defmodule Towerops.Snmp.Discovery do {:ok, sensors} end - @spec discover_vlans(Client.connection_opts(), profile()) :: {:ok, [map()]} + @spec discover_vlans( + Client.connection_opts(), + Base | {:yaml, %{required(:name) => term(), optional(atom()) => term()}} + ) :: {:ok, [map()]} defp discover_vlans(client_opts, {:yaml, _profile}) do # Dynamic profiles use Base VLAN discovery {:ok, vlans} = Base.discover_vlans(client_opts) @@ -1147,6 +1157,9 @@ defmodule Towerops.Snmp.Discovery do @spec sync_ip_addresses(Device.t(), [map()]) :: :ok @doc """ Sync IP addresses from discovery/polling results to database. + + Accepts any map with `:device_id` and `:interfaces` keys; callers in the + agent channel pass lightweight maps instead of full Device structs. """ def sync_ip_addresses(device, discovered_ip_addresses) do # Build a map of if_index to interface ID from the device's interfaces diff --git a/lib/towerops/snmp/mib_translator.ex b/lib/towerops/snmp/mib_translator.ex index 5e769c88..6ca49c7b 100644 --- a/lib/towerops/snmp/mib_translator.ex +++ b/lib/towerops/snmp/mib_translator.ex @@ -27,6 +27,10 @@ defmodule Towerops.Snmp.MibTranslator do iex> MibTranslator.translate("INVALID-MIB::badObject") {:error, :translation_failed} """ + # The Rust NIF stub returns a narrow type but at runtime the real NIF returns + # either a binary OID on success or {:error, reason} on failure. Dialyzer sees + # only the stub's narrow type, causing a false guard_fail on the is_binary clause. + @dialyzer {:nowarn_function, translate: 1} @spec translate(String.t()) :: {:ok, String.t()} | {:error, :translation_failed} def translate(mib_name) when is_binary(mib_name) do cond do diff --git a/lib/towerops/snmp/profiles/base.ex b/lib/towerops/snmp/profiles/base.ex index 26c5305b..73589941 100644 --- a/lib/towerops/snmp/profiles/base.ex +++ b/lib/towerops/snmp/profiles/base.ex @@ -298,8 +298,7 @@ defmodule Towerops.Snmp.Profiles.Base do This is the universal baseline sensor discovery that works across many vendors. Returns a list of sensor maps. """ - @spec discover_sensors(Client.connection_opts()) :: - {:ok, [Discovery.sensor_data()]} | {:error, term()} + @spec discover_sensors(Client.connection_opts()) :: {:ok, [Discovery.sensor_data()]} def discover_sensors(client_opts) do # Try to walk the sensor type OID to see if device supports ENTITY-SENSOR-MIB case Client.walk(client_opts, @entity_sensor_oids.ent_phys_sensor_type) do diff --git a/lib/towerops/snmp/profiles/dynamic.ex b/lib/towerops/snmp/profiles/dynamic.ex index c3697840..fee88bf7 100644 --- a/lib/towerops/snmp/profiles/dynamic.ex +++ b/lib/towerops/snmp/profiles/dynamic.ex @@ -448,7 +448,8 @@ defmodule Towerops.Snmp.Profiles.Dynamic do Collects all discovered sensor data for debugging purposes. Includes YAML-defined sensors (processors, state, count, table) and wireless sensors. """ - @spec collect_vendor_debug_data(map(), Client.connection_opts()) :: map() + @spec collect_vendor_debug_data(map(), Client.connection_opts()) :: + %{discovered_sensors: [map()], wireless_sensors: map()} def collect_vendor_debug_data(profile, client_opts) do # Collect all sensors using the same discovery logic {:ok, all_sensors} = discover_sensors(profile, client_opts) diff --git a/lib/towerops/snmp/profiles/vendors/arista.ex b/lib/towerops/snmp/profiles/vendors/arista.ex index 47596981..f9352254 100644 --- a/lib/towerops/snmp/profiles/vendors/arista.ex +++ b/lib/towerops/snmp/profiles/vendors/arista.ex @@ -138,7 +138,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.Arista do @spec dom_power_sensor?(String.t()) :: boolean() defp dom_power_sensor?(descr), do: Regex.match?(~r/(DOM|Xcvr) (R|T)x Power/i, descr) - @spec do_convert_dom_power(map()) :: map() + @spec do_convert_dom_power(%{required(:sensor_descr) => term(), optional(atom()) => term()}) :: map() defp do_convert_dom_power(%{sensor_type: "power", last_value: watts} = sensor) when is_number(watts) and watts > 0 do # Convert watts to dBm: dBm = 10 * log10(watts * 1000) dbm_value = 10 * :math.log10(watts * 1000) diff --git a/lib/towerops/snmp/profiles/vendors/powervault.ex b/lib/towerops/snmp/profiles/vendors/powervault.ex index fadf7574..d349ea30 100644 --- a/lib/towerops/snmp/profiles/vendors/powervault.ex +++ b/lib/towerops/snmp/profiles/vendors/powervault.ex @@ -129,7 +129,18 @@ defmodule Towerops.Snmp.Profiles.Vendors.Powervault do end # Parse voltage sensor: "12.1V" - @spec parse_voltage(String.t(), String.t(), String.t(), String.t()) :: map() | nil + @spec parse_voltage(String.t(), String.t(), String.t(), String.t()) :: + nil + | %{ + sensor_type: String.t(), + sensor_descr: String.t(), + sensor_unit: String.t(), + sensor_oid: String.t(), + sensor_index: String.t(), + last_value: float(), + sensor_divisor: 1, + metadata: %{vendor: String.t(), message_type: String.t()} + } defp parse_voltage(oid, index, name, value_part) do if String.contains?(name, "Voltage") do # Match pattern: "12.1V" @@ -155,7 +166,18 @@ defmodule Towerops.Snmp.Profiles.Vendors.Powervault do end # Parse current sensor: "0.5A" - @spec parse_current(String.t(), String.t(), String.t(), String.t()) :: map() | nil + @spec parse_current(String.t(), String.t(), String.t(), String.t()) :: + nil + | %{ + sensor_type: String.t(), + sensor_descr: String.t(), + sensor_unit: String.t(), + sensor_oid: String.t(), + sensor_index: String.t(), + last_value: float(), + sensor_divisor: 1, + metadata: %{vendor: String.t(), message_type: String.t()} + } defp parse_current(oid, index, name, value_part) do if String.contains?(name, "Current") do # Match pattern: "0.5A" @@ -181,7 +203,21 @@ defmodule Towerops.Snmp.Profiles.Vendors.Powervault do end # Parse battery charge sensor: "95%" - @spec parse_charge(String.t(), String.t(), String.t(), String.t()) :: map() | nil + @spec parse_charge(String.t(), String.t(), String.t(), String.t()) :: + nil + | %{ + sensor_type: String.t(), + sensor_descr: String.t(), + sensor_unit: String.t(), + sensor_oid: String.t(), + sensor_index: String.t(), + last_value: integer(), + sensor_divisor: 1, + high_limit: 100, + low_warn_limit: 20, + low_limit: 10, + metadata: %{vendor: String.t(), message_type: String.t()} + } defp parse_charge(oid, index, name, value_part) do if String.contains?(name, "Charge") or String.contains?(name, "Battery") do # Match pattern: "95%" diff --git a/lib/towerops/snmp/profiles/vendors/registry.ex b/lib/towerops/snmp/profiles/vendors/registry.ex index 85948163..c88697fd 100644 --- a/lib/towerops/snmp/profiles/vendors/registry.ex +++ b/lib/towerops/snmp/profiles/vendors/registry.ex @@ -185,7 +185,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.Registry do @doc """ List all registered vendor modules. """ - @spec list_vendors() :: [module()] + @spec list_vendors() :: [module(), ...] def list_vendors, do: @vendors @doc """ diff --git a/lib/towerops/snmp/profiles/vendors/unifi.ex b/lib/towerops/snmp/profiles/vendors/unifi.ex index 46adb31f..c3263736 100644 --- a/lib/towerops/snmp/profiles/vendors/unifi.ex +++ b/lib/towerops/snmp/profiles/vendors/unifi.ex @@ -55,7 +55,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.Unifi do end end - @spec fetch_channels(Client.connection_opts()) :: {:ok, map()} | {:error, term()} + @spec fetch_channels(Client.connection_opts()) :: {:ok, %{String.t() => term()}} | {:error, term()} defp fetch_channels(client_opts) do Client.walk(client_opts, "1.3.6.1.4.1.41112.1.6.1.2.1.4") end diff --git a/lib/towerops/sonar/client.ex b/lib/towerops/sonar/client.ex index 56cbacde..8e36058f 100644 --- a/lib/towerops/sonar/client.ex +++ b/lib/towerops/sonar/client.ex @@ -171,8 +171,6 @@ defmodule Towerops.Sonar.Client do end end - defp get_retry_after(_headers), do: 60 - defp parse_retry_after(value) when is_binary(value) do case Integer.parse(value) do {seconds, _} -> seconds diff --git a/lib/towerops/topology.ex b/lib/towerops/topology.ex index b12b5b70..754d5c4c 100644 --- a/lib/towerops/topology.ex +++ b/lib/towerops/topology.ex @@ -1159,7 +1159,7 @@ defmodule Towerops.Topology do Returns {:ok, count} where count is the number of neighbors discovered. """ - @spec discover_lldp_neighbors(String.t()) :: {:ok, integer()} | {:error, term()} + @spec discover_lldp_neighbors(String.t()) :: {:ok, non_neg_integer()} | {:error, :device_not_found} def discover_lldp_neighbors(device_id) do case Lldp.discover_neighbors(device_id) do {:ok, %{neighbors: neighbors}} -> @@ -1528,7 +1528,6 @@ defmodule Towerops.Topology do end end - defp classify_utilization_level(nil), do: nil defp classify_utilization_level(pct) when pct < 30, do: :low defp classify_utilization_level(pct) when pct < 60, do: :medium defp classify_utilization_level(pct) when pct < 80, do: :high diff --git a/lib/towerops/uisp/sync.ex b/lib/towerops/uisp/sync.ex index 9ddf24a9..ec324efb 100644 --- a/lib/towerops/uisp/sync.ex +++ b/lib/towerops/uisp/sync.ex @@ -98,14 +98,8 @@ defmodule Towerops.Uisp.Sync do end defp safe_sync(label, fun) do - case fun.() do - {:ok, result} -> - result - - {:error, reason} -> - Logger.warning("UISP #{label} sync failed: #{inspect(reason)}") - %{error: reason} - end + {:ok, result} = fun.() + result rescue e -> Logger.warning("UISP #{label} sync crashed: #{Exception.message(e)}") @@ -114,8 +108,6 @@ defmodule Towerops.Uisp.Sync do defp humanize_sync_error(:unauthorized), do: "Authentication failed — check your API key" - defp humanize_sync_error(:timeout), do: "Connection timed out — UISP may be temporarily unavailable" - defp humanize_sync_error(reason) when is_binary(reason) do if String.contains?(reason, ["CA trust store", "cacert", "certificate"]) do "SSL certificate verification failed — unable to establish a secure connection" diff --git a/lib/towerops/workers/alert_notification_worker.ex b/lib/towerops/workers/alert_notification_worker.ex index d0d23029..5af36dae 100644 --- a/lib/towerops/workers/alert_notification_worker.ex +++ b/lib/towerops/workers/alert_notification_worker.ex @@ -32,10 +32,6 @@ defmodule Towerops.Workers.AlertNotificationWorker do {:error, :device_not_found} -> Logger.warning("Device not found for alert #{alert_id}") :ok - - {:error, reason} -> - Logger.error("Failed to send trigger notification for alert #{alert_id}: #{inspect(reason)}") - {:error, reason} end end @@ -55,10 +51,6 @@ defmodule Towerops.Workers.AlertNotificationWorker do {:error, :alert_not_found} -> Logger.warning("Alert #{alert_id} not found for acknowledge notification") :ok - - {:error, reason} -> - Logger.error("Failed to send acknowledge notification for alert #{alert_id}: #{inspect(reason)}") - {:error, reason} end end @@ -78,10 +70,6 @@ defmodule Towerops.Workers.AlertNotificationWorker do {:error, :alert_not_found} -> Logger.warning("Alert #{alert_id} not found for resolve notification") :ok - - {:error, reason} -> - Logger.error("Failed to send resolve notification for alert #{alert_id}: #{inspect(reason)}") - {:error, reason} end end diff --git a/lib/towerops/workers/device_poller_worker.ex b/lib/towerops/workers/device_poller_worker.ex index ce95a603..3fb8023b 100644 --- a/lib/towerops/workers/device_poller_worker.ex +++ b/lib/towerops/workers/device_poller_worker.ex @@ -860,19 +860,7 @@ defmodule Towerops.Workers.DevicePollerWorker do end) # Batch insert all sensor readings - case Snmp.create_sensor_readings_batch(reading_entries) do - {:ok, _count} -> - :ok - - {:error, reason} -> - require Logger - - Logger.error( - "Failed to batch insert sensor readings: #{inspect(reason)}", - count: length(reading_entries), - error: reason - ) - end + {_count, _} = Snmp.create_sensor_readings_batch(reading_entries) # Process metadata updates and change detection individually Enum.each(poll_results, fn {sensor, result} -> @@ -1341,9 +1329,11 @@ defmodule Towerops.Workers.DevicePollerWorker do end defp build_oper_status_event(interface, current_attrs, device_id, timestamp) do - # Handle nil status values from SNMP failures + # Handle nil status values from SNMP failures. current_attrs.if_oper_status + # is always set (parse_if_status/1 covers all inputs) — only the stored + # interface field can be nil before the first successful poll. old_status = interface.if_oper_status || "unknown" - new_status = current_attrs.if_oper_status || "unknown" + new_status = current_attrs.if_oper_status message = if interface.if_oper_status do diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 352209a2..3005dd02 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -62,6 +62,11 @@ defmodule ToweropsWeb.AgentChannel do @typep base64_string :: String.t() @typep protobuf_binary :: binary() + # These helpers pass lightweight maps (not full Device structs) to + # Discovery.sync_* functions; the sync functions only need a few fields + # (`:id`, `:device_id`, `:interfaces`), so this is safe at runtime. + @dialyzer {:nowarn_function, [process_ip_addresses: 3, process_processors: 2, process_storage: 2]} + # Heartbeat timeout: close channel if no heartbeat in 5 minutes @heartbeat_timeout_seconds 300 # Check heartbeat every 2 minutes @@ -571,14 +576,6 @@ defmodule ToweropsWeb.AgentChannel do binary_size: byte_size(binary_b64) ) - {:noreply, socket} - - {:error, :base64_decode_failed} -> - Logger.error("Failed to decode credential test result (invalid base64)", - agent_token_id: socket.assigns.agent_token_id, - binary_size: byte_size(binary_b64) - ) - {:noreply, socket} end end @@ -617,14 +614,6 @@ defmodule ToweropsWeb.AgentChannel do binary_size: byte_size(binary_b64) ) - {:noreply, socket} - - {:error, :base64_decode_failed} -> - Logger.error("Failed to decode ping result (invalid base64)", - agent_token_id: socket.assigns.agent_token_id, - binary_size: byte_size(binary_b64) - ) - {:noreply, socket} end end @@ -662,14 +651,6 @@ defmodule ToweropsWeb.AgentChannel do binary_size: byte_size(binary_b64) ) - {:noreply, socket} - - {:error, :base64_decode_failed} -> - Logger.error("Failed to decode check result (invalid base64)", - agent_token_id: socket.assigns.agent_token_id, - binary_size: byte_size(binary_b64) - ) - {:noreply, socket} end end @@ -711,14 +692,6 @@ defmodule ToweropsWeb.AgentChannel do binary_size: byte_size(binary_b64) ) - {:noreply, socket} - - {:error, :base64_decode_failed} -> - Logger.error("Failed to decode LLDP topology result (invalid base64)", - agent_token_id: socket.assigns.agent_token_id, - binary_size: byte_size(binary_b64) - ) - {:noreply, socket} end end @@ -767,14 +740,6 @@ defmodule ToweropsWeb.AgentChannel do binary_size: byte_size(binary_b64) ) - {:noreply, socket} - - {:error, :base64_decode_failed} -> - Logger.error("Failed to decode SNMP result (invalid base64)", - agent_token_id: socket.assigns.agent_token_id, - binary_size: byte_size(binary_b64) - ) - {:noreply, socket} end end diff --git a/lib/towerops_web/controllers/api/v1/geoip_controller.ex b/lib/towerops_web/controllers/api/v1/geoip_controller.ex index 2a70a463..a861d30c 100644 --- a/lib/towerops_web/controllers/api/v1/geoip_controller.ex +++ b/lib/towerops_web/controllers/api/v1/geoip_controller.ex @@ -59,14 +59,6 @@ defmodule ToweropsWeb.Api.V1.GeoipController do conn |> put_status(:internal_server_error) |> json(%{error: reason}) - - {:error, reason} -> - # Internal error - log details but return generic message - Logger.error("GeoIP batch import failed: #{inspect(reason)}") - - conn - |> put_status(:internal_server_error) - |> json(%{error: "Import failed"}) end end end diff --git a/lib/towerops_web/live/graph_live/show.ex b/lib/towerops_web/live/graph_live/show.ex index be48b2fd..a53baec1 100644 --- a/lib/towerops_web/live/graph_live/show.ex +++ b/lib/towerops_web/live/graph_live/show.ex @@ -1181,7 +1181,7 @@ defmodule ToweropsWeb.GraphLive.Show do ip: device.ip_address, community: snmp_config.community, version: snmp_config.version, - port: device.snmp_port || 161, + port: device.snmp_port, # Increased for live polling - devices may be slow to respond timeout: 3000 ] diff --git a/lib/towerops_web/live/insights_live/index.ex b/lib/towerops_web/live/insights_live/index.ex index 444f96e8..c06db850 100644 --- a/lib/towerops_web/live/insights_live/index.ex +++ b/lib/towerops_web/live/insights_live/index.ex @@ -99,17 +99,13 @@ defmodule ToweropsWeb.InsightsLive.Index do if ids == [] do {:noreply, socket} else - case Preseem.dismiss_insights(ids) do - {:ok, count} -> - {:noreply, - socket - |> assign(:selected_ids, MapSet.new()) - |> load_insights() - |> put_flash(:info, "#{count} insight(s) dismissed")} + {:ok, count} = Preseem.dismiss_insights(ids) - {:error, _} -> - {:noreply, put_flash(socket, :error, t("Failed to dismiss insights"))} - end + {:noreply, + socket + |> assign(:selected_ids, MapSet.new()) + |> load_insights() + |> put_flash(:info, "#{count} insight(s) dismissed")} end end diff --git a/lib/towerops_web/live/org/preseem_insights_live.ex b/lib/towerops_web/live/org/preseem_insights_live.ex index 593e11ce..e48c72e6 100644 --- a/lib/towerops_web/live/org/preseem_insights_live.ex +++ b/lib/towerops_web/live/org/preseem_insights_live.ex @@ -101,17 +101,13 @@ defmodule ToweropsWeb.Org.PreseemInsightsLive do if ids == [] do {:noreply, socket} else - case Preseem.dismiss_insights(ids) do - {:ok, count} -> - {:noreply, - socket - |> assign(:selected_ids, MapSet.new()) - |> load_insights() - |> put_flash(:info, "#{count} insight(s) dismissed")} + {:ok, count} = Preseem.dismiss_insights(ids) - {:error, _} -> - {:noreply, put_flash(socket, :error, t("Failed to dismiss insights"))} - end + {:noreply, + socket + |> assign(:selected_ids, MapSet.new()) + |> load_insights() + |> put_flash(:info, "#{count} insight(s) dismissed")} end end diff --git a/lib/towerops_web/live/site_live/form.ex b/lib/towerops_web/live/site_live/form.ex index b4912265..5d12ef86 100644 --- a/lib/towerops_web/live/site_live/form.ex +++ b/lib/towerops_web/live/site_live/form.ex @@ -112,9 +112,6 @@ defmodule ToweropsWeb.SiteLive.Form do {:error, reason} when is_binary(reason) -> {:noreply, put_flash(socket, :error, t("Geocoding failed: %{reason}", reason: reason))} - - {:error, _} -> - {:noreply, put_flash(socket, :error, t("Geocoding failed. Please check the address and try again."))} end else {:noreply, put_flash(socket, :error, t("Please enter an address to geocode"))}