signalk-server icon indicating copy to clipboard operation
signalk-server copied to clipboard

Block deltas with null or missing path

Open tkurki opened this issue 1 year ago • 4 comments

Plugins consuming deltas may not implement error handling properly or just deliberately choose to log deltas / pathvalues with missing paths, causing support requests and annoyance.

I think it would be a good idea to discard them outright or have a configurable option to do so. Along with enough but not continuous logging to find the culprit.

tkurki avatar Sep 21 '22 17:09 tkurki

One source of this bug seems to be here, as the third parameter to setDelta is path: https://github.com/SignalK/signalk-server/blob/0c357f367c128b87f284a14ddab587e25df74d58/src/deltaeditor.ts#L55-L59

When the base deltas are sent, such as uuid, the path field does not contain a period (the path itself is "uuid"). That then sends a pathless delta out, stripping out the path by explicitly setting it to '' on line 59 after plugins have been registered. I think this guarantees plugins that are registered on startup encounter pathless deltas at least once. I don't know enough about metadata deltas yet to know the implications of changing that back to path, but it does resolve all pathless deltas on startup as far as I can tell.

vadian avatar Feb 12 '23 16:02 vadian

Not sure if this is applicable, but these default deltas are all without a '.' in their path: communication mmsi name

anhorbc avatar Feb 17 '23 20:02 anhorbc

@anhorbc that's super helpful! @tkurki do you think we should repath these as vessels.self.X or similar, and see if that resolves the issue? At the very least, that would make for more elegant filtering of empty path values.

vadian avatar Feb 17 '23 20:02 vadian

Sorry, I think you're barking up the wrong tree here. This issue is not about deltas whose path is the empty string, that is by design.

Instead it this is about value deltas (as opposed to meta deltas) that have in their values array

  • pathvalues with no path { value: 3.14}
  • pathvalues path as null { path: null, value: 3.14} that cause plugin/webapp/client code to blow up or just log a lot.

tkurki avatar Feb 21 '23 17:02 tkurki