fix(animation) get/setKeyframes throws when animation isn't defined.

[chrisgervang]

Requesting keyframes for animations that aren't defined shouldn't throw, instead a more graceful behavior of returning undefined (for get) or doing nothing (for set) is preferred.

[coveralls]

Pull Request Test Coverage Report for Build 1685551732

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 20 of 20 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+2.1%) to 28.149%

---

Totals
Change from base Build 1685540636:	2.1%
Covered Lines:	257
Relevant Lines:	791

---

💛 - Coveralls

[hkim8567]

It seems like for both cases(existing or non-existing animation), setKeyframes' behavior is identical. Would it be possible for code to get some feedback or differentiate if it couldn't find the animation?

[chrisgervang]

It seems like for both cases(existing or non-existing animation), setKeyframes' behavior is identical. Would it be possible for code to get some feedback or differentiate if it couldn't find the animation?

In normal runtime, I don't think feedback is useful and can effect performance. There's a logger used in other vis.gl libraries that has levels, and I think it'd be great to print that it couldn't be found at a debug level of 3 or 4. I'll take a note to add this once that logger is working in hubble (I ran into integration issues).

[hkim8567]

I'm not too familiar with how to get/setKeyframes are being used, but curious to know why it's not ideal to throw an error. If I tried and set animation, and I got a wrong animation id that resulted in getting/SetKeyframes not working, I would want instant feedback on that so I can go back and change my animation id.

[chrisgervang]

They may be used in an async context where the getter/setter call may be statically defined, but the animation won't be set until the data loads. In that case, returning undefined or doing nothing is useful behavior in the final product.

I do think getting some feedback is important, so we can wait to merge until the logger is in place, but I don't typically throw errors unless it's completely necessary. Most JS devs check for undefined.
Deck's error handling system doesn't throw errors by default, since a partially working scene is better than a crashed program. https://deck.gl/docs/api-reference/core/deck#onerror
I think something like this would be a good in-between. It gives feedback without crashing the program.