Merge pull request #11405 from lafirest/fix/kal_kialo

fix(calendar): make date parse error reason more sense

apps/emqx/src/emqx_calendar.erl

@@ -158,9 +158,9 @@ offset_second_(Offset) when is_list(Offset) ->
             _ ->
                 error({bad_time_offset, Offset})
         end,
-    Hour = list_to_int_or_error(HourStr, {bad_time_offset_hour, HourStr}),
+    Hour = str_to_int_or_error(HourStr, {bad_time_offset_hour, HourStr}),
-    Minute = list_to_int_or_error(MinuteStr, {bad_time_offset_minute, MinuteStr}),
+    Minute = str_to_int_or_error(MinuteStr, {bad_time_offset_minute, MinuteStr}),
-    Second = list_to_int_or_error(SecondStr, {bad_time_offset_second, SecondStr}),
+    Second = str_to_int_or_error(SecondStr, {bad_time_offset_second, SecondStr}),
     (Hour =< 23) orelse error({bad_time_offset_hour, Hour}),
     (Minute =< 59) orelse error({bad_time_offset_minute, Minute}),
     (Second =< 59) orelse error({bad_time_offset_second, Second}),
@@ -417,7 +417,11 @@ do_parse_date_str(Date, [Key | Formatter], Result) ->
     <<DatePart:Size/binary-unit:8, Tail/binary>> = Date,
     case lists:member(Key, ?DATE_PART) of
         true ->
-            do_parse_date_str(Tail, Formatter, Result#{Key => erlang:binary_to_integer(DatePart)});
+            %% Note: Here is a fix to make the error reason more sense
+            %% when the format or date can't be matched,
+            %% but the root reason comment underneath at <ROOT>
+            PartValue = str_to_int_or_error(DatePart, bad_formatter_or_date),
+            do_parse_date_str(Tail, Formatter, Result#{Key => PartValue});
         false ->
             case lists:member(Key, ?DATE_ZONE_NAME) of
                 true ->
@@ -425,6 +429,13 @@ do_parse_date_str(Date, [Key | Formatter], Result) ->
                         parsed_offset => offset_second(DatePart)
                     });
                 false ->
+                    %% <ROOT>
+                    %% Here should have compared the date part with the key,
+                    %% but for compatibility, we can't fix it here
+                    %% e.g.
+                    %% date_to_unix_ts('second','%Y-%m-%d %H-%M-%S', '2022-05-26 10:40:12')
+                    %% this is valid in 4.x, but actually '%H-%M-%S' can't match with '10:40:12'
+                    %% We cannot ensure whether there are more exceptions in the user's rule
                     do_parse_date_str(Tail, Formatter, Result)
             end
     end.
@@ -456,10 +467,12 @@ dm(10) -> 273;
 dm(11) -> 304;
 dm(12) -> 334.
 
-list_to_int_or_error(Str, Error) ->
+str_to_int_or_error(Str, Error) ->
     case string:to_integer(Str) of
         {Int, []} ->
             Int;
+        {Int, <<>>} ->
+            Int;
         _ ->
             error(Error)
     end.

apps/emqx_rule_engine/test/emqx_rule_funcs_SUITE.erl

@@ -1031,6 +1031,35 @@ timezone_to_offset_seconds_helper(FunctionName) ->
     apply_func(FunctionName, [local]),
     ok.
 
+t_parse_date_errors(_) ->
+    ?assertError(
+        bad_formatter_or_date,
+        emqx_rule_funcs:date_to_unix_ts(
+            second, <<"%Y-%m-%d %H:%M:%S">>, <<"2022-059999-26 10:40:12">>
+        )
+    ),
+    ?assertError(
+        bad_formatter_or_date,
+        emqx_rule_funcs:date_to_unix_ts(second, <<"%y-%m-%d %H:%M:%S">>, <<"2022-05-26 10:40:12">>)
+    ),
+
+    %% Compatibility test
+    UnixTs = 1653561612,
+    ?assertEqual(
+        UnixTs,
+        emqx_rule_funcs:date_to_unix_ts(second, <<"%Y-%m-%d %H:%M:%S">>, <<"2022-05-26 10:40:12">>)
+    ),
+
+    ?assertEqual(
+        UnixTs,
+        emqx_rule_funcs:date_to_unix_ts(second, <<"%Y-%m-%d %H-%M-%S">>, <<"2022-05-26 10:40:12">>)
+    ),
+
+    ?assertEqual(
+        UnixTs,
+        emqx_rule_funcs:date_to_unix_ts(second, <<"%Y-%m-%d %H:%M:%S">>, <<"2022-05-26 10-40-12">>)
+    ).
+
 %%------------------------------------------------------------------------------
 %% Utility functions
 %%------------------------------------------------------------------------------

changes/ce/perf-11405.en.md

@@ -0,0 +1 @@
+Improve the error reason of the `date_to_unix_ts` to make more sense.