[amazonechocontrol] Improve handling of throttled notification polls - #21439
[amazonechocontrol] Improve handling of throttled notification polls#21439ML19821 wants to merge 5 commits into
Conversation
Amazon reports the actual failure cause in the x-amzn-ErrorType header
(for /api/notifications: "ThrottlingException", body {"message":"Rate
exceeded"}, see openhab#19781), while the status line only carries a generic
"Bad Request". The header is now part of the exception message, and the
cookie-free response body is logged at DEBUG, capped at 512 characters
and only while TRACE is off - previously the body was only visible at
TRACE, which also dumps session cookies.
A response identified as throttled - status 429, or an x-amzn-ErrorType
starting with "ThrottlingException" - additionally fails fast instead of
going through the generic three retries with a fixed 2 s pause: those
all land inside the same limit window, and every retry is itself a
counted request that keeps the account throttled. FailMode.NORMAL keeps
its behaviour, it never retried in the first place.
The classification and the assembly of the failure reason are extracted
into two package-private static methods, so both can be tested without a
live response. The reason phrase is treated as optional, because a
response is not required to carry one.
Also adds the missing return after the "Request aborted." completion:
the second completeExceptionally() on a status of 0 was a no-op.
Related to openhab#19781, openhab#19714, openhab#21146.
AI-assisted-by: Claude Code
Signed-off-by: Martin Littkovsky <2018turtle@proton.me>
A failed notifications poll was indistinguishable from "no notifications set": getNotifications() swallowed the exception and returned an empty list, which set every next* channel to UNDEF (openhab#20018) and left nextRefreshNotifications at Long.MAX_VALUE, silently disabling the event-driven refresh until the next hourly cycle. The failure now propagates - getNotifications() is the only one of the swallowing list getters around it whose empty result is published as a statement about the device, the others feed command options and per-device states - and the caller distinguishes transient from sustained failure: the last known state survives short outages, and from the third consecutive failure onwards the next* channels go UNDEF, because at that point the binding genuinely does not know the state anymore. The update is repeated on every further failure, so an echo handler that registers during the outage is told as well. Retries back off from 300 s, doubling up to the regular 3600 s refresh interval and resetting on success; a flat retry interval measurably keeps a throttled account throttled. The third failure is therefore reached about 15 minutes after the first. refreshNotifications() also returns early while the backoff is running, so the push path (PUSH_NOTIFICATION_CHANGE) can no longer bypass it. Failure count, delay and the resulting deadline are one state and live together in NotificationPollBackoff behind one lock, because the poll is reached both from the polling job and from a Jetty I/O thread: with the deadline in a separate caller field, a poll succeeding on one thread and a poll failing on the other could each write one half and leave a backoff that never became due again. nextRefreshNotifications now carries only the success path's wake-up time. A re-login clears the backoff, so failures caused by the expired session do not keep the poll silent after the user has fixed it. Failures are logged once per streak at WARN, repeats at DEBUG, the transition to UNDEF at WARN and recovery at INFO. Related to openhab#20018, openhab#21146. AI-assisted-by: Claude Code Signed-off-by: Martin Littkovsky <2018turtle@proton.me>
|
@ML19821 , would love to try this JAR and be a beta tester for it. I have a very heavy implementation of Amazon binding which has been broken for over 1.5 years now with many open bug reports from me on it. What makes me unique is I have 15 Echos, 10 Tiles (grabs x/y locations from them) and 9 smart home devices hanging off this Amazon account. A lot of the features of the Echo's don't work anymore such as LAST SPOKEN TTS and Tile Locations. I worked a lot with @mgeramb when it was developed originally. Best, Jay |
|
@ML19821 Thanks for picking this up. It was also on my todo list. |
Reduce the comments introduced by the two previous commits to short notes on actual constraints, as requested in review. No functional change; all changed lines are comments. Signed-off-by: Martin Littkovsky <2018turtle@proton.me> AI-assisted-by: Claude Code
|
Done before your review (same as on #21429/#21382): inline comments trimmed to the binding's norm ( |
|
@jaywiseman1971 Happy to have a tester with a setup that size — here is a build: https://github.com/ML19821/openhab-addons/releases/tag/amazonechocontrol-throttling-test-1 (5.2.2-SNAPSHOT for openHAB 5.2.x; uninstall the official binding first, then drop the jar into your addons folder). Honest expectations: this PR makes throttling visible (real cause in the log), stops the binding from hammering a throttled endpoint, and turns the |
|
@ML19821 would you be interested in fixing some of the amazonechocontrol issues? I'm sure @jaywiseman1971 will be more then willing to test (many times). I'm sure there will be others too. The amazonechocontrol binding could use some love and i'm happy to assist where i can. |
Response bodies stay at TRACE; the DEBUG line keeps the metadata that matters, the x-amzn-ErrorType header and the status. AI-assisted-by: Claude Code Signed-off-by: Martin Littkovsky <2018turtle@proton.me>
|
Done proactively, same as on the netatmo PR: the failure body is out of the DEBUG line (bodies stay at TRACE), the |
I uninstalled the GUI one from OH 5.1.4 and dropped in yours but it required this below, so I had an OLD version of this back in the day (velocity-engine-core-2.3.jar) and dropped it in and it came online. Unresolved requirement: Import-Package: org.apache.velocity; version="[2.3.0,3.0.0)" Will provide feedback soon along with a debug log so you can see what is happening. 
Thank you again! |
|
@ML19821, did a clean OH startup and got this error. Will continue to monitor. 2026-08-19 16:01:36.913 [WARN ] [.amazonechocontrol.internal.handler.AccountHandler] - Failed to get notifications for amazonechocontrol:account:account2, next attempt in 300 s: https://na-api-alexa.amazon.com/api/notifications failed with code 400: Bad Request (x-amzn-ErrorType: ThrottlingException:http://internal.amazon.com/coral/com.amazon.coral.availability/) |
|
@jaywiseman1971 One question that decides what helps you most: do you have any of the
Either way, to keep expectations honest: neither build touches |
Yes, all my echo's have nextAlarm and nextReminder and I reference nextAlarm every night to remind you that you don't have an alarm set. Best, Jay |
|
@ML19821, this is positive. The last 2 tiles I linked to Amazon account (many months ago) where ALWAYS in an UNKNOWN state, now they are not. Not saying they are working w/o me leaving the house yet, but at least they are online now. 
Best, Jay |
|
@lsiepel Happy to take you up on that. I'll start with two that scratch my own itches: #18085 (the volume-then-speak race — my own hallway announcement suffers from exactly that) and #21426 (the push-stream parse errors; same shape as the shelly NotifyEvent fix — one unreadable field should not take down the whole message). Full disclosure, since you rightly asked for clarity about AI involvement: this is very much a human-plus-Claude-Code operation. Claude does the heavy lifting, I do the testing on a real installation and take the blame 😉 — my own Java would not survive your review. So far the combination seems to work, but please keep the review bar exactly where it is. I'll open issues/PRs as things actually get ready rather than promising timelines — and with @jaywiseman1971 volunteering to test, the feedback loop is covered. |
|
@ML19821, the lastvoicecommand is still not working, see below, actually have 16 echo's, LOL Will start a debug and see if that grabs anything worth while. 
Best, Jay |
|
@ML19821, is there a place I can drop 7 x 10 mb amazon trace logs zipped up to you vs. on this forum since the logs have confidential information about my setup in it? I used these TRACE commands. log:set TRACE org.openhab.binding.amazonechocontrol |
|
@ML19821, the nextalarm hasn't worked either for quite some time. With this new build, same results. 
|
|
@jaywiseman1971 Good call not posting them publicly — one important reason on top of your own: TRACE logs of this binding contain your Amazon session cookies, so treat them like a password. You can send them to the address on my commits: 2018turtle@proton.me. Proton accepts up to 50 MB of attachments per incoming mail, and mail encoding inflates files somewhat in transit — so split the seven zips across three mails (2–3 zips each) to be safe. I will treat them as confidential and delete them after analysis; if you want to be thorough, log the binding account out and back in afterwards so the old session cookies in the logs become worthless. On |
|
@ML19821, this confirms that the Tiles aren't getting updated still with new build. Lower priority than last voice command. I'm away from home most of the day today, will send zipped logs to u later. They all zip up to 5 mb total. 
Best, Jay |
lsiepel
left a comment
lsiepel
left a comment
There was a problem hiding this comment.
This is an initial AI-assisted review.
Two comments to look at
| if (!connection.isLoggedIn()) { | ||
| return; | ||
| } | ||
| if (notificationPollBackoff.shouldSkip(System.currentTimeMillis())) { |
There was a problem hiding this comment.
The backoff check does not reserve an in-flight poll. checkLoginAndData() invokes this method on the scheduler while PUSH_NOTIFICATION_CHANGE invokes it directly on the Jetty callback thread, so both can pass shouldSkip() before either request reports a result. This can send multiple requests during the same throttling window and advance the failure counter several steps at once, or let a racing success reset a throttling failure. Please serialize notification polls or model an in-flight attempt atomically, and add a concurrent-trigger regression test.
There was a problem hiding this comment.
Good catch. Polls are now reserved atomically (acdd539): NotificationPollBackoff.tryStart() admits a single running attempt and hands out a token identifying it, and onSuccess()/onFailure() end the attempt under the same lock. A caller refused while an attempt is running does not lose its trigger: the refusal is remembered under that lock and handed to whoever ends the attempt — on success the handler schedules an immediate catch-up poll (the finished attempt's response may predate the pushed change), on failure it expires, because a push must not undercut the backoff deadline. The timer checks in checkLoginAndData() only keep the once-per-second tick quiet; admission is decided solely by tryStart(). Deterministic tests cover the refusal, catch-up on success, expiry on failure, and idempotence of a double refusal.
| nextLoginCheck = 0; | ||
| nextDataRefresh = 0; | ||
| // failures of the expired session must not delay polls on the new one | ||
| notificationPollBackoff.reset(); |
There was a problem hiding this comment.
This reset can be undone by an older request that is still in flight. setConnection() runs from the servlet thread, while a scheduled or push-triggered refreshNotifications() may already be waiting on the previous Connection; when that call later fails, its catch block invokes onFailure() and installs a backoff for the newly authenticated session. Please invalidate results belonging to the old connection, for example with a captured connection generation, and cover re-login during an active poll with a regression test.
There was a problem hiding this comment.
Also fixed, via the same token (acdd539): setConnection() releases the running attempt so the new session may poll immediately, and a late result carrying the old token is discarded — its failure cannot throttle the new session, its success cannot confirm it or delay its first poll. Late failure, late success and the release-on-reset are each covered by a test.
|
@jaywiseman1971 please continue in the right issue to troubleshoot. This PR is about improving the visibility, not the issue itself. |
This has been sent to your email and I pasted the THING Echo that I asked the questions to cross reference the echo used. Best, Jay |
…a replaced connection Signed-off-by: Martin Littkovsky <2018turtle@proton.me> AI-assisted-by: Claude Code
3 participants
Description
On some accounts the hourly notifications poll (
/api/notifications) fails on every cycle with400: Bad Request(#21146, #19714). The real cause is throttling: Amazon answers withx-amzn-ErrorType: ThrottlingExceptionand{"message":"Rate exceeded"}, but the binding never reads that header — and a failed poll was indistinguishable from "no notifications set" (#20018).Changes
x-amzn-ErrorTypeis appended to the exception message and logged at DEBUG; response bodies stay at TRACE.429, or aThrottlingExceptionheader) instead of burning the generic retries — every retry is itself a counted request that keeps the account throttled.FailMode.NORMALis unaffected.next*channels goUNDEF. Retries back off exponentially (300 s doubling up to the hourly refresh), the push path honours the backoff, failures log once per streak.Testing
148 unit tests (28 new): the backoff ladder, the
UNDEFthreshold, the throttling classification, no-second-request on a throttled fail-fast, and the token/serialization behavior of the review round. SAT report clean againstmain, re-established on the current revision. Verified on a production installation (build of 2026-08-19, before the review round's concurrency rework — that rework's behavior is pinned by the added unit tests): the ladder runs to the second,UNDEFappears after the third failure, no idle polling. Measured over two days: more than 100 attempts at spacings from seconds up to a deliberate 10.7-hour silence — every one answered400 ThrottlingException. For such an account the endpoint is effectively permanently locked; the value of this PR there is the honest diagnosis, the strongly reduced request pressure, and channels that sayUNDEFinstead of pretending "no alarm set". Occasionally-throttled accounts additionally gain the transient-failure tolerance.Fixes the misleading diagnosis of #21146 / #19714; related: #19781, #20018 (on a permanently throttled account the
next*channels still end upUNDEF— but nowUNDEFmeans what it says).A follow-up PR will skip this poll entirely while no notification channel is linked.
Transparency: this patch was developed with AI assistance (Claude); every commit carries an
AI-assisted-bytrailer. All changes were built, tested and verified on a production system by the author.