SmartThingsCommunity / SmartThingsEdgeDrivers

Apache License 2.0
269 stars 462 forks source link

fix: Avoid a nil index crash when data loss occurs #1620

Closed dljsjr closed 2 months ago

dljsjr commented 2 months ago

Check all that apply

Type of Change

Checklist

Description of Change

This fixes an edge case where we were assuming that a persistent field on a device was always present; while this should more or less hold true, there are times when it won't. In those cases, the driver would crash.

Drivers in this state would already be nonfunctional, but we want to avoid chewing up resources with a hot crash loop.

Summary of Completed Tests

I tested this locally by onboarding my Hue setup and then deliberaly taking actions that would lead to the persisted fields getting blown away; the driver is nonfunctional, but no longer hits a hot crash loop.

github-actions[bot] commented 2 months ago

Test Results

   62 files    383 suites   0s :stopwatch: 1 862 tests 1 862 :white_check_mark: 0 :zzz: 0 :x: 3 249 runs  3 249 :white_check_mark: 0 :zzz: 0 :x:

Results for commit b5cc4871.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 2 months ago

Minimum allowed coverage is 90%

Generated by :monkey: cobertura-action against b5cc4871177c422411f4ceef9cbd85056d5fe005

varzac commented 2 months ago

It's not obvious to me where the assumed key was causing a npe. Can you clarify where exactly that was happening?

dljsjr commented 2 months ago

@varzac It was a missing IP address, actually. This block was the cause of the original crash report: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/pull/1620/files#diff-5a04f9b727d9d403a969e8b24dff2d9162b47cd213254faf3f910be2697c24c4L252

The Application Key isn't the only "important" piece of data that can be lost, but it's the only one that's not easy to recover.

The other changes in this PR address other locations where we were trying to access Fields that could have been lost. They presented as additional crashes or pathological/resource consuming code paths that showed up after I fixed the initial crash.

github-actions[bot] commented 2 months ago

Channel deleted.