Handle errors related to KeySignature class instance methods

[meganindya]

On branch musicutils, TypeScript ported file keySignature.ts has some of this methods including the constructor throw errors.

Handle with try catch calls to all such instances in other files.

[alfredhb]

This looks like it's still open, I'll add these try catch where keySignature is used. Is there a specific error handling pattern / method desired?

[meganindya]

Yeah pls go ahead. Not anything in particular really — simple try { ... } catch (e) { ... }. Perhaps, look for a few code snippets to see how they've been used.

Do follow these guidelines though.

[alfredhb]

Regarding uses of KeySignature, the only use in other files I found which wasn't wrapped in a try catch was its instantiation in currentPitch.ts (not including the py files in archive which I assume can be left as-is). I've added the try catch block to this instantiation and ensured that tests still pass. and linting does as well

Would you like me to add tests for this? I'm struggling to think of tests to add without mocking the KeySignature constructor to force it to throw an error

[meganindya]

There are 11 methods (one of which is the constructor and another is a setter) which throw errors. The objective is to do a try-catch handling where error cases assign default values. The rest will throw errors anyway, like in the constructor of CurrentPitch - if KeySignature cannot be instantiated, CurrentPitch won't be instantiated.

However, the bulk of this issue is w.r.t. the test file corresponding to KeySignature in tests/. Please perform an error checking for all such methods which can throw error, using expect(...).toThrowError(...) if any case is missing. Also, wrap KeySignature instantiation in try-catch. I'm not sure, but sometimes jest reports a failure if all error possibilities aren't handled.

This is a rough idea, please report a list of what you think needs to be done re this issue in a comment ... I'll review that and you could then create a PR?

[alfredhb]

Hey sorry, something came up, and I'll need to step away from this issue at this time I apologise.

[nk183]

@meganindya, is this issue still there?

[meganindya]

yeah go ahead.

[KrishavRajSingh]

All I need to do is to add try-catch block on each function of keySignature.ts?