Skip to content

Server: keep tuyalisten thread alive if send_discovery_request fails#726

Merged
jasonacox merged 1 commit into
jasonacox:masterfrom
mschlenstedt:fix-tuyalisten-discovery-thread
Jul 19, 2026
Merged

Server: keep tuyalisten thread alive if send_discovery_request fails#726
jasonacox merged 1 commit into
jasonacox:masterfrom
mschlenstedt:fix-tuyalisten-discovery-thread

Conversation

@mschlenstedt

Copy link
Copy Markdown
Contributor

Problem

scanner.send_discovery_request() in the tuyalisten loop of server/server.py runs outside any try/except. A single transient error (e.g. an OSError from a short network outage) kills the UDP listener thread for port 7000 (UDPPORTAPP) permanently — the exception propagates out of the thread function and the thread never comes back.

Since protocol 3.5 devices only announce themselves in response to these periodic discovery requests, they are never (re)discovered again until the server is restarted. Devices already in deviceslist keep working, but any 3.5 device that powers up later (or was offline when the thread died) stays undiscovered: /status/{id} returns an error for it and the web UI (e.g. device_dps.html) shows no data, while 3.3 devices keep working normally. This makes the failure hard to diagnose — the server looks healthy otherwise.

We hit this in production (LoxBerry TinyTuya plugin): after a nightly maintenance window, both 3.5 devices vanished from the server while the 3.3 devices kept publishing; a restart fixed it instantly.

Fix

Wrap the call in try/except and log the error, so the thread survives transient failures and simply retries on the next BROADCASTTIME cycle.

Test

Fault injection (monkeypatched send_discovery_request raising OSError on calls 2 and 3):

  • Without the patch: Exception in thread Thread-3 (tuyalisten) after the 2nd call, no further discovery requests are sent, 3.5 devices are lost after the next power cycle.
  • With the patch: both failures are logged (Failed to send discovery request on port 7000: [Errno 101] ...), the thread keeps running and discovery resumes on the next cycle.

🤖 Generated with Claude Code

scanner.send_discovery_request() in the tuyalisten loop runs outside any
try/except. A single transient error (e.g. OSError from a short network
outage) kills the UDP listener thread for port 7000 permanently. Since
protocol 3.5 devices only announce themselves in response to these
discovery requests, they are never (re)discovered again until the server
is restarted - /status/{id} then returns an error for them while 3.3
devices keep working.

Wrap the call in try/except and log the error, so the thread survives
transient failures and retries on the next BROADCASTTIME cycle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@jasonacox-sam jasonacox-sam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch — this is a real reliability bug.

The send_discovery_request() call in the tuyalisten loop (lines 332–335) runs without any exception handling, so a single transient OSError from a network hiccup propagates out of the thread function and kills it permanently. Since protocol 3.5 devices only announce themselves in response to these periodic discovery broadcasts, losing this thread means any 3.5 device that powers on later (or was briefly offline) is invisible until a full server restart. The 3.3 devices keep working because they're discovered via the other UDP listener, which makes the failure silent and hard to diagnose — exactly what you hit in production.

The fix is tight: broad except Exception is correct here because this is a daemon thread that must not die, the error is logged (not swallowed), and last_broadcast is still updated so the next retry waits for the next BROADCASTTIME cycle rather than hammering. Good.

One minor observation: if send_discovery_request starts failing persistently (not just transiently), the thread will log the same error every BROADCASTTIME cycle indefinitely with no escalation. Not a blocker for this PR — just worth keeping in mind if anyone ever adds server health/alerting.

Approving. Thanks @mschlenstedt for the clear write-up and the repro.

— Sam ⚙️

@jasonacox

Copy link
Copy Markdown
Owner

@mschlenstedt did you mean to close this?

@mschlenstedt

Copy link
Copy Markdown
Contributor Author

No, its just a comment that we have to wait for this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the TinyTuya server’s UDP discovery listener loop so transient failures during periodic v3.5 discovery broadcasts don’t permanently terminate the tuyalisten thread (port UDPPORTAPP), restoring reliable rediscovery behavior after brief network outages.

Changes:

  • Wrap scanner.send_discovery_request() in tuyalisten() with try/except to prevent the listener thread from dying on transient exceptions.
  • Add an error log entry when discovery request sending fails, allowing the loop to continue and retry on subsequent broadcast cycles.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server/server.py
Comment on lines +337 to +341
except Exception as err:
# A transient error (e.g. network hiccup) must not kill this
# listener thread - otherwise 3.5 devices are never discovered
# again until the server is restarted.
log.error("Failed to send discovery request on port %d: %s", port, err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think leaving this one as-is is the better tradeoff for this PR. The goal here is to keep the discovery thread alive regardless of what send_discovery_request() throws, and the existing error log is enough to surface that something went wrong without turning a persistent failure into a traceback every broadcast cycle. If we want richer diagnostics for unexpected exceptions later, that feels like a follow-up observability change rather than a blocker for this reliability fix.

— Sam ⚙️

@jasonacox jasonacox left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the PR diff and the discussion thread. This is a focused reliability fix for the v3.5 discovery path in server/server.py: wrapping the periodic scanner.send_discovery_request() call keeps the tuyalisten() daemon thread alive after transient send failures, while still logging the error and preserving the existing retry cadence via last_broadcast.

I also checked the inline discussion about the broad except Exception. I agree with the current tradeoff here: this is a thread boundary where the primary requirement is that discovery must not die permanently, and logging every failure without tracebacks avoids noisy repeated stack traces if the network is persistently unavailable. Narrower diagnostics can be a follow-up observability improvement if needed, but I do not see it as a blocker for this PR.

Validation performed against the PR head (e9699fa):

  • python -m tests passed: 38 tests, 1 skipped.
  • python -m py_compile server/server.py passed.
  • python -m pylint -E server/server.py passed.

No blocking findings from my review. Ready to merge.

@jasonacox
jasonacox merged commit 47b43f8 into jasonacox:master Jul 19, 2026
33 checks passed
@jasonacox

Copy link
Copy Markdown
Owner

Thanks for this @mschlenstedt ! Good addition. 🙏

jasonacox pushed a commit that referenced this pull request Jul 19, 2026
Wrap send_discovery_request() in try/except so transient OSErrors no
longer permanently kill the UDP listener thread. Protocol 3.5 devices
are now rediscovered after network interruptions without a restart.

Closes #726. Credit: @mschlenstedt.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jasonacox

Copy link
Copy Markdown
Owner

New container: jasonacox/tinytuya:1.20.0p17

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.

4 participants