You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
No log line, no exception, no status code — the response object is discarded. A caller sees None and has no way to learn whether the server was down, rejected its credentials, or returned a 500. Worse, the falsy return is easy to conflate with "this node has no systems" (which returns []), so the common for s in node.discover_systems() or []: idiom turns an auth failure into a silent zero-iteration loop.
This is the read-side twin of #42: same "failure becomes indistinguishable from an empty success" shape, on GET rather than POST.
Reproduce
node=Node(protocol="http", address="localhost", port=8282,
username="wrong", password="wrong")
print(node.discover_systems()) # None — no indication it was a 401
Point at a stopped server for the same result via a different cause.
Possible fix
Two defensible options; pick one and apply it consistently to every discovery method:
Log and keep returning None — lower-churn, preserves the documented return contract (the docstring already says "or None if the HTTP request failed"), but still leaves callers unable to react programmatically.
At minimum the failure must be logged at ERROR with the status code and response text, so it isn't invisible. Worth auditing System.discover_datastreams() / discover_controlstreams() at the same time — they call res.json() without checking res.ok at all, so a failed listing raises a JSON decode error rather than anything meaningful.
Sub-issue of #46.
Problem
Node.discover_systems()(node.py:316-338) returnsNonewhen the HTTP request fails:No log line, no exception, no status code — the response object is discarded. A caller sees
Noneand has no way to learn whether the server was down, rejected its credentials, or returned a 500. Worse, the falsy return is easy to conflate with "this node has no systems" (which returns[]), so the commonfor s in node.discover_systems() or []:idiom turns an auth failure into a silent zero-iteration loop.This is the read-side twin of #42: same "failure becomes indistinguishable from an empty success" shape, on GET rather than POST.
Reproduce
Point at a stopped server for the same result via a different cause.
Possible fix
Two defensible options; pick one and apply it consistently to every discovery method:
raise ResourceDiscoveryError(...)carrying status code and body, once Typed exception hierarchy instead of bare Exception #47 lands the hierarchy.None— lower-churn, preserves the documented return contract (the docstring already says "orNoneif the HTTP request failed"), but still leaves callers unable to react programmatically.At minimum the failure must be logged at ERROR with the status code and response text, so it isn't invisible. Worth auditing
System.discover_datastreams()/discover_controlstreams()at the same time — they callres.json()without checkingres.okat all, so a failed listing raises a JSON decode error rather than anything meaningful.