From 82800faadf3debe9db5f26493b660b875624cc25 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Tue, 28 May 2024 18:07:14 +0200 Subject: [PATCH 1/3] fix: make protobuf schema decode error reported to user less cryptic Before this commit the user would see a cryptic warning log including function_clause when decoding a message failed. This commit improves this by removing function_caluse as well as some other cryptic words from the message and adding a description describing what went wrong. Fixes: https://emqx.atlassian.net/browse/EMQX-12453 --- .../src/emqx_schema_registry.app.src | 2 +- .../src/emqx_schema_registry_serde.erl | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/emqx_schema_registry/src/emqx_schema_registry.app.src b/apps/emqx_schema_registry/src/emqx_schema_registry.app.src index f05295579..efd9f1162 100644 --- a/apps/emqx_schema_registry/src/emqx_schema_registry.app.src +++ b/apps/emqx_schema_registry/src/emqx_schema_registry.app.src @@ -1,6 +1,6 @@ {application, emqx_schema_registry, [ {description, "EMQX Schema Registry"}, - {vsn, "0.3.0"}, + {vsn, "0.3.1"}, {registered, [emqx_schema_registry_sup]}, {mod, {emqx_schema_registry_app, []}}, {included_applications, [ diff --git a/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl b/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl index 638147fd1..96c5816ba 100644 --- a/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl +++ b/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl @@ -64,7 +64,21 @@ handle_rule_function(sparkplug_encode, [Term | MoreArgs]) -> [?EMQX_SCHEMA_REGISTRY_SPARKPLUGB_SCHEMA_NAME, Term | MoreArgs] ); handle_rule_function(schema_decode, [SchemaId, Data | MoreArgs]) -> - decode(SchemaId, Data, MoreArgs); + try + decode(SchemaId, Data, MoreArgs) + catch + error:{gpb_error, {decoding_failure, {_Data, _Schema, {error, function_clause, _Stack}}}} -> + throw( + {schema_decode_error, #{ + error_type => decoding_failure, + schema_id => SchemaId, + data => Data, + more_args => MoreArgs, + msg => + <<"The given data could not be decoded. Please check the input data and the schema.">> + }} + ) + end; handle_rule_function(schema_decode, Args) -> error({args_count_error, {schema_decode, Args}}); handle_rule_function(schema_encode, [SchemaId, Term | MoreArgs]) -> From 9d7acc6a3289f34d449fe3b7e473ce13ae44c306 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Tue, 28 May 2024 18:24:45 +0200 Subject: [PATCH 2/3] docs: add change log entry --- changes/ee/fix-13147.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ee/fix-13147.en.md diff --git a/changes/ee/fix-13147.en.md b/changes/ee/fix-13147.en.md new file mode 100644 index 000000000..4f717892d --- /dev/null +++ b/changes/ee/fix-13147.en.md @@ -0,0 +1 @@ +Error messages for decoding failures in the rule engine protobuf decode functions have been improved by adding a clear descriptions to indicate what went wrong when message decoding fails. From 9b089a4e5aa44c3013743a325002c21d1a0e5bc0 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Wed, 29 May 2024 10:20:15 +0200 Subject: [PATCH 3/3] fix: use explain instead of msg as error explanation and add test --- .../src/emqx_schema_registry_serde.erl | 2 +- .../test/emqx_schema_registry_SUITE.erl | 20 ++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl b/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl index 96c5816ba..e8da35449 100644 --- a/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl +++ b/apps/emqx_schema_registry/src/emqx_schema_registry_serde.erl @@ -74,7 +74,7 @@ handle_rule_function(schema_decode, [SchemaId, Data | MoreArgs]) -> schema_id => SchemaId, data => Data, more_args => MoreArgs, - msg => + explain => <<"The given data could not be decoded. Please check the input data and the schema.">> }} ) diff --git a/apps/emqx_schema_registry/test/emqx_schema_registry_SUITE.erl b/apps/emqx_schema_registry/test/emqx_schema_registry_SUITE.erl index 22252b7c3..d9286f266 100644 --- a/apps/emqx_schema_registry/test/emqx_schema_registry_SUITE.erl +++ b/apps/emqx_schema_registry/test/emqx_schema_registry_SUITE.erl @@ -44,7 +44,8 @@ sparkplug_tests() -> t_sparkplug_decode, t_sparkplug_encode, t_sparkplug_decode_encode_with_message_name, - t_sparkplug_encode_float_to_uint64_key + t_sparkplug_encode_float_to_uint64_key, + t_decode_fail ]. init_per_suite(Config) -> @@ -532,6 +533,23 @@ t_encode(Config) -> end, ok. +t_decode_fail(_Config) -> + SerdeName = my_serde, + SerdeType = protobuf, + ok = create_serde(SerdeType, SerdeName), + Payload = <<"ss">>, + ?assertThrow( + {schema_decode_error, #{ + data := <<"ss">>, + error_type := decoding_failure, + explain := _, + more_args := [<<"Person">>], + schema_id := <<"my_serde">> + }}, + emqx_rule_funcs:schema_decode(<<"my_serde">>, Payload, <<"Person">>) + ), + ok. + t_decode(Config) -> SerdeType = ?config(serde_type, Config), SerdeName = my_serde,