Prevent crashes on ambiguity when casting timezones (#3663)

* Fall back to UTC when timezone gap found

* bugfix 1

* bugfix 2

* Apply safe tz conversion to all cases where it's done on a ts other than now

---------

Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>

lib/plausible/site.ex

@@ -6,6 +6,7 @@ defmodule Plausible.Site do
   import Ecto.Changeset
   alias Plausible.Auth.User
   alias Plausible.Site.GoogleAuth
+  alias Plausible.Timezones
 
   @type t() :: %__MODULE__{}
 
@@ -208,9 +209,7 @@ defmodule Plausible.Site do
 
   def local_start_date(site) do
     site.stats_start_date
-    |> Timex.Timezone.convert("UTC")
-    |> Timex.Timezone.convert(site.timezone)
-    |> Timex.to_date()
+    |> Timezones.to_date_in_timezone(site.timezone)
   end
 
   defp clean_domain(changeset) do

lib/plausible/stats/base.ex

@@ -2,6 +2,7 @@ defmodule Plausible.Stats.Base do
   use Plausible.ClickhouseRepo
   use Plausible
   alias Plausible.Stats.{Query, Filters}
+  alias Plausible.Timezones
   import Ecto.Query
 
   @no_ref "Direct / None"
@@ -513,15 +514,13 @@ defmodule Plausible.Stats.Base do
     {:ok, first} = NaiveDateTime.new(date_range.first, ~T[00:00:00])
 
     first_datetime =
-      Timex.to_datetime(first, site.timezone)
-      |> Timex.Timezone.convert("UTC")
+      first
+      |> Timezones.to_utc_datetime(site.timezone)
       |> beginning_of_time(site.native_stats_start_at)
 
     {:ok, last} = NaiveDateTime.new(date_range.last |> Timex.shift(days: 1), ~T[00:00:00])
 
-    last_datetime =
-      Timex.to_datetime(last, site.timezone)
-      |> Timex.Timezone.convert("UTC")
+    last_datetime = Timezones.to_utc_datetime(last, site.timezone)
 
     {first_datetime, last_datetime}
   end

lib/plausible/stats/clickhouse.ex

@@ -7,6 +7,7 @@ defmodule Plausible.Stats.Clickhouse do
   import Ecto.Query, only: [from: 2]
 
   alias Plausible.Stats.Query
+  alias Plausible.Timezones
 
   @no_ref "Direct / None"
 
@@ -27,9 +28,7 @@ defmodule Plausible.Stats.Clickhouse do
         nil
 
       _ ->
-        Timex.Timezone.convert(datetime, "UTC")
-        |> Timex.Timezone.convert(site.timezone)
-        |> DateTime.to_date()
+        Timezones.to_date_in_timezone(datetime, site.timezone)
     end
   end
 
@@ -585,15 +584,14 @@ defmodule Plausible.Stats.Clickhouse do
     {:ok, first} = NaiveDateTime.new(date_range.first, ~T[00:00:00])
 
     first_datetime =
-      Timex.to_datetime(first, site.timezone)
-      |> Timex.Timezone.convert("UTC")
+      first
+      |> Timezones.to_utc_datetime(site.timezone)
       |> beginning_of_time(site.native_stats_start_at)
 
     {:ok, last} = NaiveDateTime.new(date_range.last |> Timex.shift(days: 1), ~T[00:00:00])
 
     last_datetime =
-      Timex.to_datetime(last, site.timezone)
-      |> Timex.Timezone.convert("UTC")
+      Timezones.to_utc_datetime(last, site.timezone)
 
     {first_datetime, last_datetime}
   end

lib/plausible/timezones.ex

@@ -6,6 +6,36 @@ defmodule Plausible.Timezones do
     |> Enum.sort_by(& &1[:offset], :desc)
   end
 
+  @spec to_utc_datetime(NaiveDateTime.t(), String.t()) :: DateTime.t()
+  def to_utc_datetime(naive_date_time, timezone) do
+    case Timex.to_datetime(naive_date_time, timezone) do
+      %DateTime{} = tz_dt ->
+        Timex.Timezone.convert(tz_dt, "UTC")
+
+      %Timex.AmbiguousDateTime{after: after_dt} ->
+        Timex.Timezone.convert(after_dt, "UTC")
+
+      {:error, {:could_not_resolve_timezone, _, _, _}} ->
+        Timex.Timezone.convert(naive_date_time, "UTC")
+    end
+  end
+
+  @spec to_date_in_timezone(Date.t() | NaiveDateTime.t() | DateTime.t(), String.t()) :: Date.t()
+  def to_date_in_timezone(dt, timezone) do
+    utc_dt = Timex.Timezone.convert(dt, "UTC")
+
+    case Timex.Timezone.convert(utc_dt, timezone) do
+      %DateTime{} = tz_dt ->
+        Timex.to_date(tz_dt)
+
+      %Timex.AmbiguousDateTime{after: after_dt} ->
+        Timex.to_date(after_dt)
+
+      {:error, {:could_not_resolve_timezone, _, _, _}} ->
+        dt
+    end
+  end
+
   defp build_option(timezone_code, acc, now) do
     case Timex.Timezone.get(timezone_code, now) do
       %Timex.TimezoneInfo{} = timezone_info ->

test/plausible_web/controllers/api/stats_controller/main_graph_test.exs

@@ -888,6 +888,22 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
       assert 2 == Enum.sum(comparison_plot)
     end
 
+    test "bugfix: don't crash when timezone gap occurs", %{conn: conn, user: user} do
+      site = insert(:site, members: [user], timezone: "America/Santiago")
+
+      populate_stats(site, [
+        build(:pageview, timestamp: relative_time(minutes: -5))
+      ])
+
+      conn =
+        get(
+          conn,
+          "/api/stats/#{site.domain}/main-graph?period=custom&from=2022-09-11&to=2022-09-21&date=2023-03-15&with_imported=true"
+        )
+
+      assert %{"plot" => _} = json_response(conn, 200)
+    end
+
     test "does not return imported data when with_imported is set to false when comparing", %{
       conn: conn,
       site: site

test/plausible_web/controllers/api/stats_controller/regions_test.exs

@@ -73,5 +73,25 @@ defmodule PlausibleWeb.Api.StatsController.RegionsTest do
       assert resp = response(conn, 400)
       assert resp =~ "Failed to parse 'to' argument."
     end
+
+    test "bugfix: don't crash on ambiguous date time", %{conn: conn, user: user} do
+      # The site has timezone set to Azores.
+      # Given it's 28th Nov and there's 30 day range, the starting day falls on 29th Oct
+      # which coincides with daylight savings time change there:
+      # https://www.timeanddate.com/time/change/portugal/ponta-delgada-azores.
+      site = insert(:site, members: [user], timezone: "Atlantic/Azores")
+
+      populate_stats(site, [
+        build(:pageview, timestamp: relative_time(minutes: -5))
+      ])
+
+      conn =
+        get(
+          conn,
+          "/api/stats/#{site.domain}/regions?period=30d&date=2023-11-28&with_imported=true"
+        )
+
+      assert json_response(conn, 200)
+    end
   end
 end