python garmin_fit_sdk is masking KeyboardInterrupt exceptions

pwr 14 days ago

Using the python garmin_fit_sdk I discovered some weird behavior that would be nice to fix. Not sure where to file a bug report so I'm doing it here...!

Observed behavior:

Pressing ctrl-c while parsing a file results in the parsing of the file silently 'succeeding', returning partial file contents up to the point that parsing was stopped. Nothing is returned in 'errors' and no python KeyboardInterrupt is raised

Expected behavior:

Pressing ctrl-c while parsing a file should either report an error in 'errors', or propagate the KeyboardInterrupt out to the caller

Why it matters:

Running a script that parses a large number of .FIT files - if something appears to be going awry partway through I should be able to use normal means (KeyboardInterrupt) to stop things instead of a) letting a long process run to completion or b) resorting to extraordinary measures to stop things (i.e. kill whole process)

Likely root cause:

Looking at decoder.py on GitHub, there are many "try-except" blocks with a bare "except:" and further handling that 'swallows' the error. A bare "except" catches *all* exceptions including exceptions that are subclasses of BaseException like KeyboardInterrupt and SystemExit. (see: https://stackoverflow.com/questions/54948548/what-is-wrong-with-using-a-bare-except )

Suggested fix:

Suggest replacing "except:" with "except Exception:" in these cases.

This will propagate 'special' BaseExceptions like KeyboardInterrupt and SystemExit out to the caller (arguably more-pythonic behavior)

Will still handle all other Exception subtypes identically to current behavior.

Cheers, thx!

0 Ben FIT 14 days ago

The existing error handling, where the SDK swallows the exceptions and returns the messages up to the point of the exception, is very intentional. Most corrupt FIT files are not corrupt because they were encoded wrong, rather they are corrupt because something happened to the file downstream i.e. the file was truncated, or a few bytes were somehow corrupted in transit. Depending on what byte in a file is corrupt the exception could be anything, so any error messages that the SDK would produce would be red herrings. Swallowing the KeyboardInterrupt and SystemExit was not intended.

The outermost read() method of the Decoder is catching Exceptions, with a capital E, and should let KeyboardInterrupt and SystemExit exceptions propagate; assuming the timing is such that the code was not in one of the inner methods catching bare exceptions when you pressed CTRL-C.

We can test changing the catches and if all the tests pass then we will consider making this change. We probably won’t look at this until next year.

0 Ben FIT 1 day ago in reply to Ben FIT

We will have a fix for this in the next release which should be before EOY.