Skip to content

fix(spine): nil-check remoteFeature in SubscribeToRemote/BindToRemote (minimal patch for #87)#90

Open
terabytegmbh wants to merge 1 commit into
enbility:devfrom
synervatt:fix/subscribe-bind-nil-feature
Open

fix(spine): nil-check remoteFeature in SubscribeToRemote/BindToRemote (minimal patch for #87)#90
terabytegmbh wants to merge 1 commit into
enbility:devfrom
synervatt:fix/subscribe-bind-nil-feature

Conversation

@terabytegmbh

Copy link
Copy Markdown

Fixes #87. Minimal-scope alternative to #88.

Problem

FeatureLocal.SubscribeToRemote and BindToRemote dereference remoteDevice.FeatureByAddress() without a nil-check. When a remote device advertises a feature address that the local entity model has not yet materialized, FeatureByAddress returns nil and the subsequent .Type() / .Role() call panics inside the spine.(*events).Publish goroutine, killing the process.

Reproduced reliably in eebus-go EG-LPC use case startup: lpc.(*LPC).connected() runs Subscribe() while the remote's LoadControl feature has not yet been published.

panic: runtime error: invalid memory address or nil pointer dereference
spine.(*FeatureLocal).SubscribeToRemote
    spine/feature_local.go:451
eebus-go/features/client.(*Feature).Subscribe
eebus-go/usecases/eg/lpc.(*LPC).connected
    usecases/eg/lpc/events.go:76
created by spine.(*events).Publish

Fix

Mirror the existing nil-check pattern used immediately above (for remoteAddress.Device and remoteDevice): return a proper *model.ErrorType("feature not found") that the calling code already handles as a debug-level non-fatal event.

Scope

  • 6 production lines in spine/feature_local.go
  • 2 new test cases (TestSubscribeToRemote_NilFeature, TestBindToRemote_NilFeature)
  • No behavioural change for the non-nil path

Relation to #88

#88 from @SAY-5 also addresses #87 with the same conceptual approach (nil-check + early error return), but additionally contains a wider refactor (199 changed files, +23573/-4189 LoC). The maintainer may prefer either path:

Happy to defer to whichever direction you'd like to land — the issue itself is what matters.

Test

go test ./spine/... clean. Two new tests verify the error-returning path is taken when FeatureByAddress returns nil.

Provenance

Patch has been running in our internal synervatt/spine-go fork since 2026-04-21 without regressions in the eebus-go EG-LPC integration path.

— Thomas, Terabyte GmbH

FeatureByAddress() can return nil when a remote device advertises a
feature address that is not yet materialized in the local entity model.
The two existing call sites in FeatureLocal then call
remoteFeature.Type() / remoteFeature.Role() unconditionally, panicking
with a nil-pointer dereference. The panic happens in the goroutine
created by spine.(*events).Publish, so downstream callers cannot
recover from it.

Reproduced reliably during eebus-go EG-LPC use case startup, when
lpc.(*LPC).connected() runs Subscribe() before the remote has finished
publishing its LoadControl feature:

    panic: runtime error: invalid memory address or nil pointer dereference
    spine.(*FeatureLocal).SubscribeToRemote(...)
        spine/feature_local.go:451
    eebus-go/features/client.(*Feature).Subscribe(...)
    eebus-go/usecases/eg/lpc.(*LPC).connected(...)
        usecases/eg/lpc/events.go:76
    created by spine.(*events).Publish

Fix mirrors the existing nil-check pattern used immediately above (for
remoteAddress.Device and remoteDevice): return a proper *model.ErrorType
("feature not found") that calling code already handles as a debug-level
non-fatal event.

Includes test cases TestSubscribeToRemote_NilFeature and
TestBindToRemote_NilFeature that construct an unknown feature address
and assert no-panic + non-nil error.

Scope: 6 production lines + 2 test cases. No behavioural change for
the non-nil path.

Signed-off-by: Thomas Quien <thomas.quien@terabyte-gmbh.de>
Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nil-pointer dereference in FeatureLocal.SubscribeToRemote when remote feature address resolves to nil

1 participant