Skip to content

Commit

Permalink
feat: update error message format
Browse files Browse the repository at this point in the history
  • Loading branch information
@jean-roland
jean-roland committed Mar 18, 2024
1 parent a1a9507 commit 0c2ecd6
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 72 deletions.
21 changes: 9 additions & 12 deletions include/zenoh-pico/protocol/definitions/message.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,28 +48,25 @@
#define _Z_FRAG_BUFF_BASE_SIZE 128 // Arbitrary base size of the buffer to encode a fragment message header

// Flags:
// - T: Timestamp If T==1 then the timestamp if present
// - I: Infrastructure If I==1 then the error is related to the infrastructure else to the user
// - X: Reserved
// - E: Encoding If E==1 then the encoding is present
// - Z: Extension If Z==1 then at least one extension is present
//
// 7 6 5 4 3 2 1 0
// +-+-+-+-+-+-+-+-+
// |Z|I|T| ERR |
// |Z|E|X| ERR |
// +-+-+-+---------+
// % code:z16 %
// +---------------+
// ~ ts: <u8;z16> ~ if T==1
// % encoding %
// +---------------+
// ~ [err_exts] ~ if Z==1
// +---------------+
#define _Z_FLAG_Z_E_T 0x20
#define _Z_FLAG_Z_E_I 0x40
/// ~ pl: <u8;z32> ~ Payload
/// +---------------+
#define _Z_FLAG_Z_E_E 0x40
typedef struct {
uint16_t _code;
_Bool _is_infrastructure;
_z_timestamp_t _timestamp;
_z_encoding_t encoding;
_z_source_info_t _ext_source_info;
_z_value_t _ext_value;
_z_bytes_t _payload;
} _z_msg_err_t;
void _z_msg_err_clear(_z_msg_err_t *err);

Expand Down
69 changes: 22 additions & 47 deletions src/protocol/codec/message.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -560,42 +560,31 @@ int8_t _z_reply_decode(_z_msg_reply_t *reply, _z_zbuf_t *zbf, uint8_t header) {
int8_t _z_err_encode(_z_wbuf_t *wbf, const _z_msg_err_t *err) {
int8_t ret = _Z_RES_OK;
uint8_t header = _Z_MID_Z_ERR;
_Bool has_timestamp = _z_timestamp_check(&err->_timestamp);
if (has_timestamp) {
header |= _Z_FLAG_Z_E_T;
}
if (err->_is_infrastructure) {
header |= _Z_FLAG_Z_E_I;

// Encode header
_Bool has_encoding = err->encoding.prefix != Z_ENCODING_PREFIX_EMPTY;
if (has_encoding) {
_Z_SET_FLAG(header, _Z_FLAG_Z_E_E);
}
_Bool has_payload_ext = err->_ext_value.payload.start != NULL || err->_ext_value.encoding.prefix != 0 ||
!_z_bytes_is_empty(&err->_ext_value.encoding.suffix);
_Bool has_sinfo_ext = _z_id_check(err->_ext_source_info._id) || err->_ext_source_info._entity_id != 0 ||
err->_ext_source_info._source_sn != 0;
if (has_sinfo_ext || has_payload_ext) {
header |= _Z_FLAG_Z_Z;
if (has_sinfo_ext) {
_Z_SET_FLAG(header, _Z_FLAG_Z_Z);
}
_Z_RETURN_IF_ERR(_z_uint8_encode(wbf, header));
_Z_RETURN_IF_ERR(_z_zint_encode(wbf, err->_code));
if (has_timestamp) {
_Z_RETURN_IF_ERR(_z_timestamp_encode(wbf, &err->_timestamp));
// Encode encoding
if (has_encoding) {
_Z_RETURN_IF_ERR(_z_encoding_prefix_encode(wbf, err->encoding.prefix));
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &err->encoding.suffix));
}
// Encode extensions
if (has_sinfo_ext) {
uint8_t extheader = _Z_MSG_EXT_ENC_ZBUF | 0x01;
if (has_payload_ext) {
extheader |= _Z_MSG_EXT_FLAG_Z;
}
_Z_RETURN_IF_ERR(_z_uint8_encode(wbf, extheader));
_Z_RETURN_IF_ERR(_z_source_info_encode_ext(wbf, &err->_ext_source_info));
}
if (has_payload_ext) {
_Z_RETURN_IF_ERR(_z_uint8_encode(wbf, _Z_MSG_EXT_ENC_ZBUF | 0x02));
_Z_RETURN_IF_ERR(_z_zint_encode(wbf, _z_zint_len(err->_ext_value.encoding.prefix) +
_z_bytes_encode_len(&err->_ext_value.encoding.suffix) +
_z_bytes_encode_len(&err->_ext_value.payload)));
_Z_RETURN_IF_ERR(_z_encoding_prefix_encode(wbf, err->_ext_value.encoding.prefix));
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &err->_ext_value.encoding.suffix));
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &err->_ext_value.payload));
}
// Encode payload
_Z_RETURN_IF_ERR(_z_bytes_encode(wbf, &err->_payload));
return ret;
}
int8_t _z_err_decode_extension(_z_msg_ext_t *extension, void *ctx) {
Expand All @@ -607,13 +596,6 @@ int8_t _z_err_decode_extension(_z_msg_ext_t *extension, void *ctx) {
ret = _z_source_info_decode(&reply->_ext_source_info, &zbf);
break;
}
case _Z_MSG_EXT_ENC_ZBUF | 0x02: {
_z_zbuf_t zbf = _z_zbytes_as_zbuf(extension->_body._zbuf._val);
ret = _z_encoding_prefix_decode(&reply->_ext_value.encoding.prefix, &zbf);
ret |= _z_bytes_decode(&reply->_ext_value.encoding.suffix, &zbf);
ret |= _z_bytes_decode(&reply->_ext_value.payload, &zbf);
break;
}
default:
if (_Z_HAS_FLAG(extension->_header, _Z_MSG_EXT_FLAG_M)) {
ret = _z_msg_ext_unknown_error(extension, 0x0a);
Expand All @@ -623,23 +605,16 @@ int8_t _z_err_decode_extension(_z_msg_ext_t *extension, void *ctx) {
}
int8_t _z_err_decode(_z_msg_err_t *err, _z_zbuf_t *zbf, uint8_t header) {
*err = (_z_msg_err_t){0};
int8_t ret = _Z_RES_OK;
_z_zint_t code;
ret = _z_zint_decode(&code, zbf);
if (code <= UINT16_MAX) {
err->_code = (uint16_t)code;
} else {
ret = _Z_ERR_MESSAGE_DESERIALIZATION_FAILED;
}
err->_is_infrastructure = _Z_HAS_FLAG(header, _Z_FLAG_Z_E_I);
if ((ret == _Z_RES_OK) && _Z_HAS_FLAG(header, _Z_FLAG_Z_E_T)) {
ret = _z_timestamp_decode(&err->_timestamp, zbf);

if (_Z_HAS_FLAG(header, _Z_FLAG_Z_E_E)) {
_Z_RETURN_IF_ERR(_z_encoding_prefix_decode(&err->encoding.prefix, zbf));
_Z_RETURN_IF_ERR(_z_bytes_decode(&err->encoding.suffix, zbf));
}
if ((ret == _Z_RES_OK) && _Z_HAS_FLAG(header, _Z_FLAG_Z_Z)) {
ret = _z_msg_ext_decode_iter(zbf, _z_err_decode_extension, err);
if (_Z_HAS_FLAG(header, _Z_FLAG_Z_Z)) {
_Z_RETURN_IF_ERR(_z_msg_ext_decode_iter(zbf, _z_err_decode_extension, err));
}

return ret;
_Z_RETURN_IF_ERR(_z_bytes_decode(&err->_payload, zbf));
return _Z_RES_OK;
}

int8_t _z_ack_encode(_z_wbuf_t *wbf, const _z_msg_ack_t *ack) {
Expand Down
4 changes: 2 additions & 2 deletions src/protocol/definitions/message.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@ void _z_msg_query_clear(_z_msg_query_t *msg) {
_z_value_clear(&msg->_ext_value);
}
void _z_msg_err_clear(_z_msg_err_t *err) {
_z_timestamp_clear(&err->_timestamp);
_z_value_clear(&err->_ext_value);
_z_bytes_clear(&err->encoding.suffix);
_z_bytes_clear(&err->_payload);
}
6 changes: 3 additions & 3 deletions src/session/rx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,10 @@ int8_t _z_handle_network_message(_z_session_t *zn, _z_zenoh_message_t *msg, uint
ret = _z_trigger_reply_partial(zn, response._request_id, response._key, reply);
} break;
case _Z_RESPONSE_BODY_ERR: {
// @TODO: expose errors to the user
// @TODO: expose zenoh errors to the user
_z_msg_err_t error = response._body._err;
_z_bytes_t payload = error._ext_value.payload;
_Z_ERROR("Received Err for query %zu: code=%d, message=%.*s", response._request_id, error._code,
_z_bytes_t payload = error._payload;
_Z_ERROR("Received Err for query %zu: message=%.*s", response._request_id,
(int)payload.len, payload.start);
} break;
case _Z_RESPONSE_BODY_ACK: {
Expand Down
13 changes: 5 additions & 8 deletions tests/z_msgcodec_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1165,21 +1165,18 @@ void query_message(void) {
}

_z_msg_err_t gen_err(void) {
size_t len = 1 + gen_uint8();
return (_z_msg_err_t){
._code = gen_uint16(),
._is_infrastructure = gen_bool(),
._timestamp = gen_timestamp(),
.encoding = gen_encoding(),
._ext_source_info = gen_bool() ? gen_source_info() : _z_source_info_null(),
._ext_value = gen_bool() ? gen_value() : _z_value_null(),
._payload = gen_payload(len), // Hangs if 0
};
}

void assert_eq_err(const _z_msg_err_t *left, const _z_msg_err_t *right) {
assert(left->_code == right->_code);
assert(left->_is_infrastructure == right->_is_infrastructure);
assert_eq_timestamp(&left->_timestamp, &right->_timestamp);
assert_eq_encoding(&left->encoding, &right->encoding);
assert_eq_source_info(&left->_ext_source_info, &right->_ext_source_info);
assert_eq_value(&left->_ext_value, &right->_ext_value);
assert_eq_bytes(&left->_payload, &right->_payload);
}

void err_message(void) {
Expand Down

0 comments on commit 0c2ecd6

Please sign in to comment.