feat: Attach topic to c8y message conversion error#3217
feat: Attach topic to c8y message conversion error#3217Bravo555 merged 1 commit intothin-edge:mainfrom
Conversation
Robot Results
|
86261fc to
6d04337
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files📢 Thoughts on this report? Let us know! |
| let error = MessageConversionError { | ||
| error, | ||
| topic: input.topic.name.clone(), | ||
| }; |
There was a problem hiding this comment.
How about moving this MessageConversionError creation logic into the convert function and propagate the same type into wrap_errors and new_error_message functions as well, since the new_error_message function already has the logic below that creates the message on the error topic. That
There was a problem hiding this comment.
wrap_errors is also used in other places where we don't have a message we're converting, for example auto entity registration. Then using the topic of a message that causes auto-registration (for example some measurement) next to any errors about registration could cause confusion
There was a problem hiding this comment.
Okay, makes sense.
In that case, we can just change the signature of the new_error_message function to accept a std::error::Error instead of ConversionError so that this whole function can be shortened as:
message_or_err
.map_err(
|error| MessageConversionError {
error,
topic: input.topic.name.clone(),
})
.unwrap_or_else(|error| self.new_error_message(error))
There was a problem hiding this comment.
Ah, you're right, so I implemented your suggestion.
| let error = MessageConversionError { | ||
| error, | ||
| topic: input.topic.name.clone(), | ||
| }; |
There was a problem hiding this comment.
Okay, makes sense.
In that case, we can just change the signature of the new_error_message function to accept a std::error::Error instead of ConversionError so that this whole function can be shortened as:
message_or_err
.map_err(
|error| MessageConversionError {
error,
topic: input.topic.name.clone(),
})
.unwrap_or_else(|error| self.new_error_message(error))
Improve error reporting in c8y mapper by printing the topic when there was an error converting a message on that topic. Example: when publishing an invalid command metadata message: `$ tedge mqtt pub -r te/device/main///cmd/config_snapshot 'a'` before: `ERROR: Mapping error: expected value at line 1 column 1` after: `ERROR: Mapping error: Failed to convert a message on topic 'te/device/main///cmd/config_snapshot': expected value at line 1 column 1` Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
6d04337 to
443ec28
Compare
Proposed changes
Improve error reporting in c8y mapper by printing the topic when there was an error converting a message on that topic.
Example: when publishing an invalid command metadata message:
$ tedge mqtt pub -r te/device/main///cmd/config_snapshot 'a'before:
ERROR: Mapping error: expected value at line 1 column 1after:
ERROR: Mapping error: Failed to convert a message on topic 'te/device/main///cmd/config_snapshot': expected value at line 1 column 1Types of changes
Paste Link to the issue
Checklist
cargo fmtas mentioned in CODING_GUIDELINEScargo clippyas mentioned in CODING_GUIDELINESFurther comments