Fit file parsing error when no activities found

[thomaschampagne]

Library throws TypeError: Cannot read property 'timestamp' of undefined AT importer.fit.ts:171

File to reproduce the bug: 7b2c46-522856231.zip

[jimmykane]

Looks like an empty file :-/

Should we detect this and throw a relevant error?

[thomaschampagne]

Indeed :)

If there is not activities. Why trying to create some in the block section importer.fit.ts:168-186?

Maybe return an empty EventInterface structure? Or a null EventInterface?

When upload the fit file to strava. Strava UI returns: "The upload appears to be malformed and we are unable to process it."

[jimmykane]

It depends I think how you want to handle it. It's empty, but I think you don't want to store something empty but rather handle this as a failure.

What do you think?

For me both ways work 🙄

On Fri, 14 Feb 2020, 14:38 Thomas Champagne, notifications@github.com wrote:

Indeed :)

If there is not activities. Why trying to create some in the block section importer.fit.ts:168-186?

Maybe return an empty EventInterface structure? Or a null EventInterface?

When upload the fit file to strava. Strava UI returns: "The upload appears to be malformed and we are unable to process it."

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sports-alliance/sports-lib/issues/45?email_source=notifications&email_token=AAJVX42M6AKKNKMYTLW6443RC2NDRA5CNFSM4KVGDGV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELZBNQQ#issuecomment-586290882, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJVX4Y76HUIS3CPLNGCHILRC2NDRANCNFSM4KVGDGVQ .

[thomaschampagne]

On my side i will skip it and save nothing in db. In this case we are even unable to set the start date properly (right?).

On my side

• it can be null = ok nothing to extract in the file
• it can throw an error ideally with an "error code" to identify this case and skip the file.

The more simple could be a null EventInterface no? Except if the library has an error handler based on error codes.

Just to give you more context about this bug. I asked my community to send me a large number of files (https://twitter.com/champagnethomas/status/1227684968790462465)

I already have thousands of these files on which I make the library suffer :)

[jimmykane]

Roger that.

Would you prefer something like: new Error(Empty event)

Or new EventImportError('No data')

Something else?

I tend to avoid returning null because typically with TS, you would want to avoid multiple type returns eg: Event|null , makes the later code more conditional based.

Arguably if it was querying a DB null should be fine but now it's an operation (the import) that has to succeed or fail right? Not return "empty/null"

I think this way you could handle this error and also inform the user with a relevant message/log

[thomaschampagne]

I understand your pov with null. A specialized error will be perfect ;)

SportsLibError <-- EventError <-- EmptyEventError

I usually specialize errors by inheritance but i will fit to yours ;)

I can perform the development ! Just tell me to do it through a PR ;)

Since typescript do not support properly Error inheritance (https://github.com/Microsoft/TypeScript-wiki/blob/master/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work)

We can use something like:

class FooError extends Error {
    constructor(m: string) {
        super(m);

        // Set the prototype explicitly.
        Object.setPrototypeOf(this, FooError.prototype);
    }

    sayHello() {
        return "hello " + this.message;
    }
}

:)