Error logging in converters [DEVINFRA-1018]

[adrian-kong]

Description

Logs error, terminates the unterminating json bug

JIRA Reference

https://swift-nav.atlassian.net/browse/DEVINFRA-1018

[silverjam]

Can we do the same thing for fatal errors too?

[adrian-kong]

Can we do the same thing for fatal errors too?

yep but will need to take until inclusive or chain (?) or just panic on the filter map 🤔 which i have just opted for the panic ^^

another bug (?) that arises json2sbp never terminates i.e.

echo {"crc":10555,"length":17,"msg_type":2304,"payload":"xYZjQJhhAWwAYhDy/3v+DgA=","preamble":85,"sender":51228,"tow":1080264389,"tow_f":152,"acc_x":353,"acc_y":108,"acc_z":4194,"gyr_x":1-14,"gyr_y":-389,"gyr_z":14} | cargo run --bin json2sbp

will not terminate which might look into (?) but we have --fatal-errors option

the above might be the reason why sometimes it doesn't produce back the entire sbp file, hanging the decoder

[silverjam]

If we don't dump the buffer on EOF errors, we terminate processing but we also don't capture the error, for example:

❯ echo -e '{"msg_type":1' | cargo run --quiet --bin json2sbp --
# ... no output ...

❯ echo -e '{"msg_type":1' | cargo run --quiet --bin json2sbp -- --fatal-errors
# ... no outputs ...

Versus previously

❯ echo -e '{"msg_type":1' | cargo run --quiet --bin json2sbp --
SerdeJsonError(Error("EOF while parsing an object", line: 2, column: 0))

❯ echo -e '{"msg_type":1' | cargo run --quiet --bin json2sbp -- --fatal-errors
SerdeJsonError(Error("EOF while parsing an object", line: 2, column: 0))

[silverjam]

@adrian-kong You're change is probably correct, but it would be nice if we could detect an EOF error where we're at the true end of a stream vs an EOF in the decoder, which looks like it can also happen when we the buffer doesn't contain an entire JSON message.

[silverjam]

@adrian-kong I wonder how hard it would be to add some integration tests for the error message?

[adrian-kong]

was trying to figure out a better way instead of holding this - felt hacky but i guess it works 👍 - or maybe just eprintln inside decode, thought the dencode errors meant retry decoding so we needed two error types

[silverjam]

Agree that the solution is a bit hacky. However, since these are library functions, it didn't feel right to add eprintln there (printing errors should be an application layer behavior).