You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2020/09/30 19:56:31 UTC

[couchdb] 02/02: Fix error reporting when fetching replication filters

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch fix-replicator-filter-fetch-error
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit cfc9eeecf4da275aabb8c7d167259e3d8456a7f3
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Wed Sep 30 15:49:25 2020 -0400

    Fix error reporting when fetching replication filters
    
    Don't unnecessarily unwrap the fetch error since `error_info/1` can already
    handle the current shape. Also, make sure to translate the reason to binary for
    consistency with the other filter fetching errors in the
    `couch_replicator_filters` module.
    
    Add a test to ensure we return the `filter_fetch_error` term as that is
    explicitly turned into a 404 error in chttpd, so we try to maintain
    compatibility with CouchDB <= 3.x code.
---
 src/couch_replicator/src/couch_replicator_filters.erl   |  2 +-
 src/couch_replicator/src/couch_replicator_job.erl       |  5 ++---
 .../eunit/couch_replicator_transient_jobs_tests.erl     | 17 +++++++++++++++--
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/couch_replicator/src/couch_replicator_filters.erl b/src/couch_replicator/src/couch_replicator_filters.erl
index 50c3733..1cadce1 100644
--- a/src/couch_replicator/src/couch_replicator_filters.erl
+++ b/src/couch_replicator/src/couch_replicator_filters.erl
@@ -150,7 +150,7 @@ fetch_internal(DDocName, FilterName, Source) ->
                      couch_replicator_api_wrap:db_uri(Source),
                      couch_util:to_binary(CodeError)]
                  ),
-                 throw({fetch_error, CodeErrorMsg})
+                 throw({fetch_error, iolist_to_binary(CodeErrorMsg)})
          end
     after
         couch_replicator_api_wrap:db_close(Db)
diff --git a/src/couch_replicator/src/couch_replicator_job.erl b/src/couch_replicator/src/couch_replicator_job.erl
index ed3d00d..eaa661e 100644
--- a/src/couch_replicator/src/couch_replicator_job.erl
+++ b/src/couch_replicator/src/couch_replicator_job.erl
@@ -810,9 +810,8 @@ get_rep_id(JTx, Job, #{} = JobData) ->
     try
         couch_replicator_ids:replication_id(Rep)
     catch
-        throw:{filter_fetch_error, Error} ->
-            Error1 = io_lib:format("Filter fetch error ~p", [Error]),
-            reschedule_on_error(JTx, Job, JobData, Error1),
+        throw:{filter_fetch_error, _} = Error ->
+            reschedule_on_error(JTx, Job, JobData, {error, Error}),
             exit({shutdown, finished})
     end.
 
diff --git a/src/couch_replicator/test/eunit/couch_replicator_transient_jobs_tests.erl b/src/couch_replicator/test/eunit/couch_replicator_transient_jobs_tests.erl
index 25fc6a3..222d138 100644
--- a/src/couch_replicator/test/eunit/couch_replicator_transient_jobs_tests.erl
+++ b/src/couch_replicator/test/eunit/couch_replicator_transient_jobs_tests.erl
@@ -31,7 +31,8 @@ transient_jobs_test_() ->
                 fun teardown/1,
                 [
                     ?TDEF_FE(transient_job_is_removed, 10),
-                    ?TDEF_FE(posting_same_job_is_a_noop, 10)
+                    ?TDEF_FE(posting_same_job_is_a_noop, 10),
+                    ?TDEF_FE(transient_job_with_a_bad_filter, 10)
                 ]
             }
         }
@@ -79,7 +80,19 @@ posting_same_job_is_a_noop({Source, Target}) ->
     ?assertEqual(Pid1, Pid2),
     couch_replicator_test_helper:cancel(RepId1).
 
-   
+
+transient_job_with_a_bad_filter({Source, Target}) ->
+    DDoc = #{<<"_id">> => <<"_design/myddoc">>},
+    couch_replicator_test_helper:create_docs(Source, [DDoc]),
+    Result = couch_replicator:replicate(#{
+        <<"source">> => couch_replicator_test_helper:db_url(Source),
+        <<"target">> => couch_replicator_test_helper:db_url(Target),
+        <<"continuous">> => true,
+        <<"filter">> => <<"myddoc/myfilter">>
+    }, ?ADMIN_USER),
+    ?assertMatch({error, #{<<"error">> := <<"filter_fetch_error">>}}, Result).
+
+
 get_rep_id(Source, Target) ->
     {ok, Id, _} = couch_replicator_parse:parse_transient_rep(#{
         <<"source">> => couch_replicator_test_helper:db_url(Source),