Better Google error messages in CE (#4485)

CHANGELOG.md

@@ -36,6 +36,7 @@ All notable changes to this project will be documented in this file.
 - Sources like 'google' and 'facebook' are now stored in capitalized forms ('Google', 'Facebook') plausible/analytics#4417
 - `DATABASE_CACERTFILE` now forces TLS for PostgreSQL connections, so you don't need to add `?ssl=true` in `DATABASE_URL`
 - Change auth session cookies to token-based ones with server-side expiration management.
+- Improve Google error messages in CE plausible/analytics#4485
 
 ### Fixed
 

fixture/ga4_api_disabled_error.json

@@ -0,0 +1,27 @@
+{
+  "error": {
+    "code": 403,
+    "details": [
+      {
+        "@type": "type.googleapis.com/google.rpc.Help",
+        "links": [
+          {
+            "description": "Google developers console API activation",
+            "url": "https://console.developers.google.com/apis/api/analyticsadmin.googleapis.com/overview?project=752168887897"
+          }
+        ]
+      },
+      {
+        "@type": "type.googleapis.com/google.rpc.ErrorInfo",
+        "domain": "googleapis.com",
+        "metadata": {
+          "consumer": "projects/752168887897",
+          "service": "analyticsadmin.googleapis.com"
+        },
+        "reason": "SERVICE_DISABLED"
+      }
+    ],
+    "message": "Google Analytics Admin API has not been used in project 752168887897 before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/analyticsadmin.googleapis.com/overview?project=752168887897 then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.",
+    "status": "PERMISSION_DENIED"
+  }
+}

lib/plausible/google/ga4/http.ex

@@ -3,6 +3,8 @@ defmodule Plausible.Google.GA4.HTTP do
   HTTP client implementation for Google Analytics 4 API.
   """
 
+  use Plausible
+
   alias Plausible.HTTPClient
 
   require Logger
@@ -55,7 +57,9 @@ defmodule Plausible.Google.GA4.HTTP do
          {:ok, report} <- convert_to_maps(report) do
       {:ok, {report, row_count}}
     else
-      {:error, %{reason: %{status: 429, body: body}}} ->
+      {:error, %{reason: %{status: 429, body: body}} = error} ->
+        log_ce_error("retrieving report for #{report_request.dataset}", error)
+
         Logger.debug(
           "[#{inspect(__MODULE__)}:#{report_request.property}] Request failed for #{report_request.dataset} due to exceeding rate limit."
         )
@@ -65,7 +69,9 @@ defmodule Plausible.Google.GA4.HTTP do
         {:error,
          {:rate_limit_exceeded, dataset: report_request.dataset, offset: report_request.offset}}
 
-      {:error, %{reason: %{status: status, body: body}}} ->
+      {:error, %{reason: %{status: status, body: body}} = error} ->
+        log_ce_error("retrieving report for #{report_request.dataset}", error)
+
         Logger.debug(
           "[#{inspect(__MODULE__)}:#{report_request.property}] Request failed for #{report_request.dataset} with code #{status}: #{inspect(body)}"
         )
@@ -74,6 +80,8 @@ defmodule Plausible.Google.GA4.HTTP do
         {:error, :request_failed}
 
       {:error, reason} ->
+        log_ce_error("retrieving report for #{report_request.dataset}", reason)
+
         Logger.debug(
           "[#{inspect(__MODULE__)}:#{report_request.property}] Request failed for #{report_request.dataset}: #{inspect(reason)}"
         )
@@ -152,16 +160,20 @@ defmodule Plausible.Google.GA4.HTTP do
       {:ok, %Finch.Response{body: body, status: 200}} ->
         {:ok, body}
 
-      {:error, %HTTPClient.Non200Error{reason: %{status: 429}}} ->
+      {:error, %HTTPClient.Non200Error{reason: %{status: 429}} = error} ->
+        log_ce_error("listing accounts for user", error)
         {:error, :rate_limit_exceeded}
 
       {:error, %HTTPClient.Non200Error{} = error} when error.reason.status in [401, 403] ->
-        {:error, :authentication_failed}
+        log_ce_error("listing accounts for user", error)
+        {:error, authentication_failed(error)}
 
-      {:error, %{reason: :timeout}} ->
+      {:error, %{reason: :timeout} = error} ->
+        log_ce_error("listing accounts for user", error)
         {:error, :timeout}
 
       {:error, error} ->
+        log_ce_error("listing accounts for user", error)
         Sentry.capture_message("Error listing GA4 accounts for user", extra: %{error: error})
         {:error, :unknown}
     end
@@ -176,19 +188,25 @@ defmodule Plausible.Google.GA4.HTTP do
       {:ok, %Finch.Response{body: body, status: 200}} ->
         {:ok, body}
 
-      {:error, %HTTPClient.Non200Error{reason: %{status: 429}}} ->
+      {:error, %HTTPClient.Non200Error{reason: %{status: 429}} = error} ->
+        log_ce_error("retrieving property #{property}", error)
         {:error, :rate_limit_exceeded}
 
       {:error, %HTTPClient.Non200Error{} = error} when error.reason.status in [401, 403] ->
-        {:error, :authentication_failed}
+        log_ce_error("retrieving property #{property}", error)
+        {:error, authentication_failed(error)}
 
       {:error, %HTTPClient.Non200Error{} = error} when error.reason.status in [404] ->
+        log_ce_error("retrieving property #{property}", error)
         {:error, :not_found}
 
-      {:error, %{reason: :timeout}} ->
+      {:error, %{reason: :timeout} = error} ->
+        log_ce_error("retrieving property #{property}", error)
         {:error, :timeout}
 
       {:error, error} ->
+        log_ce_error("retrieving property #{property}", error)
+
         Sentry.capture_message("Error retrieving GA4 property #{property}",
           extra: %{error: error}
         )
@@ -245,16 +263,21 @@ defmodule Plausible.Google.GA4.HTTP do
 
         {:ok, date}
 
-      {:error, %HTTPClient.Non200Error{reason: %{status: 429}}} ->
+      {:error, %HTTPClient.Non200Error{reason: %{status: 429}} = error} ->
+        log_ce_error("retrieving #{edge} date", error)
         {:error, :rate_limit_exceeded}
 
       {:error, %HTTPClient.Non200Error{} = error} when error.reason.status in [401, 403] ->
-        {:error, :authentication_failed}
+        log_ce_error("retrieving #{edge} date", error)
+        {:error, authentication_failed(error)}
 
-      {:error, %{reason: :timeout}} ->
+      {:error, %{reason: :timeout} = error} ->
+        log_ce_error("retrieving #{edge} date", error)
         {:error, :timeout}
 
       {:error, error} ->
+        log_ce_error("retrieving #{edge} date", error)
+
         Sentry.capture_message("Error retrieving GA4 #{edge} date",
           extra: %{error: error}
         )
@@ -265,4 +288,29 @@ defmodule Plausible.Google.GA4.HTTP do
 
   defp reporting_api_url, do: "https://analyticsdata.googleapis.com"
   defp admin_api_url, do: "https://analyticsadmin.googleapis.com"
+
+  @spec authentication_failed(HTTPClient.Non200Error.t()) ::
+          {:authentication_failed, String.t() | nil}
+  defp authentication_failed(error) do
+    message =
+      case error.reason.body do
+        %{"error" => %{"message" => message}} when is_binary(message) -> message
+        _ -> nil
+      end
+
+    {:authentication_failed, message}
+  end
+
+  @spec log_ce_error(String.t(), any) :: :ok
+  defp log_ce_error(action, error)
+
+  on_ce do
+    defp log_ce_error(action, error) do
+      Logger.error("Google Analytics 4: Failed when #{action}. Reason: #{inspect(error)}")
+    end
+  end
+
+  on_ee do
+    defp log_ce_error(_action, _error), do: :ok
+  end
 end

lib/plausible_web/controllers/google_analytics_controller.ex

@@ -57,12 +57,20 @@ defmodule PlausibleWeb.GoogleAnalyticsController do
         )
         |> redirect(external: redirect_route)
 
-      {:error, :authentication_failed} ->
+      {:error, {:authentication_failed, message}} ->
-        conn
+        default_message =
-        |> put_flash(
-          :error,
           "We were unable to authenticate your Google Analytics account. Please check that you have granted us permission to 'See and download your Google Analytics data' and try again."
-        )
+
+        message =
+          if Plausible.ce?() do
+            message || default_message
+          else
+            default_message
+          end
+
+        conn
+        |> put_flash(:ttl, :timer.seconds(5))
+        |> put_flash(:error, message)
         |> redirect(external: redirect_route)
 
       {:error, :timeout} ->
@@ -128,12 +136,20 @@ defmodule PlausibleWeb.GoogleAnalyticsController do
         )
         |> redirect(external: redirect_route)
 
-      {:error, :authentication_failed} ->
+      {:error, {:authentication_failed, message}} ->
-        conn
+        default_message =
-        |> put_flash(
-          :error,
           "Google Analytics authentication seems to have expired. Please try again."
-        )
+
+        message =
+          if Plausible.ce?() do
+            message || default_message
+          else
+            default_message
+          end
+
+        conn
+        |> put_flash(:ttl, :timer.seconds(5))
+        |> put_flash(:error, message)
         |> redirect(external: redirect_route)
 
       {:error, :timeout} ->
@@ -192,12 +208,20 @@ defmodule PlausibleWeb.GoogleAnalyticsController do
         )
         |> redirect(external: redirect_route)
 
-      {:error, :authentication_failed} ->
+      {:error, {:authentication_failed, message}} ->
-        conn
+        default_message =
-        |> put_flash(
-          :error,
           "Google Analytics authentication seems to have expired. Please try again."
-        )
+
+        message =
+          if Plausible.ce?() do
+            message || default_message
+          else
+            default_message
+          end
+
+        conn
+        |> put_flash(:ttl, :timer.seconds(5))
+        |> put_flash(:error, message)
         |> redirect(external: redirect_route)
 
       {:error, :timeout} ->

test/plausible/imported/google_analytics4_test.exs

@@ -25,6 +25,10 @@ defmodule Plausible.Imported.GoogleAnalytics4Test do
                     |> Enum.map(&File.read!/1)
                     |> Enum.map(&Jason.decode!/1)
 
+  if Plausible.ce?() do
+    @moduletag :capture_log
+  end
+
   setup :verify_on_exit!
 
   describe "parse_args/1 and import_data/2" do

test/plausible_web/controllers/google_analytics_controller_test.exs

@@ -10,6 +10,10 @@ defmodule PlausibleWeb.GoogleAnalyticsControllerTest do
 
   require Plausible.Imported.SiteImport
 
+  if Plausible.ce?() do
+    @moduletag :capture_log
+  end
+
   setup :verify_on_exit!
 
   describe "GET /:website/import/google-analytics/property" do
@@ -101,6 +105,43 @@ defmodule PlausibleWeb.GoogleAnalyticsControllerTest do
                "We were unable to authenticate your Google Analytics account"
     end
 
+    @tag :ce_build_only
+    test "redirects to imports and exports on disabled API with flash error", %{
+      conn: conn,
+      site: site
+    } do
+      expect(
+        Plausible.HTTPClient.Mock,
+        :get,
+        fn _url, _opts ->
+          body = "fixture/ga4_api_disabled_error.json" |> File.read!() |> Jason.decode!()
+          {:error, %HTTPClient.Non200Error{reason: %{status: 403, body: body}}}
+        end
+      )
+
+      conn =
+        conn
+        |> get("/#{site.domain}/import/google-analytics/property", %{
+          "access_token" => "token",
+          "refresh_token" => "foo",
+          "expires_at" => "2022-09-22T20:01:37.112777"
+        })
+
+      assert redirected_to(conn, 302) ==
+               PlausibleWeb.Router.Helpers.site_path(
+                 conn,
+                 :settings_imports_exports,
+                 site.domain
+               )
+
+      assert Phoenix.Flash.get(conn.assigns.flash, :error) =~
+               """
+               Google Analytics Admin API has not been used in project 752168887897 before or it is disabled. \
+               Enable it by visiting https://console.developers.google.com/apis/api/analyticsadmin.googleapis.com/overview?project=752168887897 then retry. \
+               If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.\
+               """
+    end
+
     test "redirects to imports and exports on timeout error with flash error", %{
       conn: conn,
       site: site
@@ -391,6 +432,44 @@ defmodule PlausibleWeb.GoogleAnalyticsControllerTest do
                "Google Analytics authentication seems to have expired."
     end
 
+    @tag :ce_build_only
+    test "redirects to imports and exports on disabled API with flash error", %{
+      conn: conn,
+      site: site
+    } do
+      expect(
+        Plausible.HTTPClient.Mock,
+        :post,
+        fn _url, _opts, _params ->
+          body = "fixture/ga4_api_disabled_error.json" |> File.read!() |> Jason.decode!()
+          {:error, %HTTPClient.Non200Error{reason: %Finch.Response{status: 403, body: body}}}
+        end
+      )
+
+      conn =
+        conn
+        |> post("/#{site.domain}/import/google-analytics/property", %{
+          "property" => "properties/428685906",
+          "access_token" => "token",
+          "refresh_token" => "foo",
+          "expires_at" => "2022-09-22T20:01:37.112777"
+        })
+
+      assert redirected_to(conn, 302) ==
+               PlausibleWeb.Router.Helpers.site_path(
+                 conn,
+                 :settings_imports_exports,
+                 site.domain
+               )
+
+      assert Phoenix.Flash.get(conn.assigns.flash, :error) =~
+               """
+               Google Analytics Admin API has not been used in project 752168887897 before or it is disabled. \
+               Enable it by visiting https://console.developers.google.com/apis/api/analyticsadmin.googleapis.com/overview?project=752168887897 then retry. \
+               If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.\
+               """
+    end
+
     test "redirects to imports and exports on timeout with flash error",
          %{
            conn: conn,
@@ -573,6 +652,47 @@ defmodule PlausibleWeb.GoogleAnalyticsControllerTest do
                "Google Analytics authentication seems to have expired."
     end
 
+    @tag :ce_build_only
+    test "redirects to imports and exports on disabled API with flash error",
+         %{
+           conn: conn,
+           site: site
+         } do
+      expect(
+        Plausible.HTTPClient.Mock,
+        :get,
+        fn _url, _params ->
+          body = "fixture/ga4_api_disabled_error.json" |> File.read!() |> Jason.decode!()
+          {:error, %HTTPClient.Non200Error{reason: %{status: 403, body: body}}}
+        end
+      )
+
+      conn =
+        conn
+        |> get("/#{site.domain}/import/google-analytics/confirm", %{
+          "property" => "properties/428685906",
+          "access_token" => "token",
+          "refresh_token" => "foo",
+          "expires_at" => "2022-09-22T20:01:37.112777",
+          "start_date" => "2024-02-22",
+          "end_date" => "2024-02-26"
+        })
+
+      assert redirected_to(conn, 302) ==
+               PlausibleWeb.Router.Helpers.site_path(
+                 conn,
+                 :settings_imports_exports,
+                 site.domain
+               )
+
+      assert Phoenix.Flash.get(conn.assigns.flash, :error) =~
+               """
+               Google Analytics Admin API has not been used in project 752168887897 before or it is disabled. \
+               Enable it by visiting https://console.developers.google.com/apis/api/analyticsadmin.googleapis.com/overview?project=752168887897 then retry. \
+               If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.\
+               """
+    end
+
     test "redirects to imports and exports on timeout with flash error",
          %{
            conn: conn,