Improve server failure handling #338
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stintel
force-pushed
the
feature/server_failures
branch
2 times, most recently
from
b0220b8
to
f87554c
Compare
This would have caught the accidental changes to the git submodules in commit 3a0551e ("Dockerfile: bump ESP-ADF to support WIS auth"). As we've been aiming for no warnings, let's just turn all warnings into errors.
We will add another HTTP event callback for the ESP Audio HTTP stream, to avoid adding too much conditionals in the callback used by the audio pipeline to WIS. Let's rename the current callback function to make it more clear it is used by the HTTP stream to the API, similar to the function initializing the HTTP stream (init_ap_to_api).
ESP-IDF's http_parser component will detect the username and password in the URL, and esp_http_client will set the Authorization header if the auth type is set to basic, and the parser found a username and password in the URL. If the parser didn't find a username and password, esp_http_client will just skip setting the header entirely, so having auth type hardcoded to basic is not a problem even if users configure a URL without username and password.
And enable HTTP Basic Auth unconditionally like we do in commit
667e073feb8d ("audio: enable HTTP Basic Auth in HTTP stream to WIS").
Reduce default HTTP stream timeout from 30s to 2s. Letting a user wait longer for a failure on a voice command or TTS response is terrible UX.
With the existing callbacks for the HTTP streams we can only capture HTTP response codes. If a problem occurs on a lower layer, this doesn't trigger these callbacks. Add an extra callback for the HTTP stream audio element, and use an enum to distinguish which audio element sent the callback. When an error occurs, play the error tone from SPIFFS, write an error message on the display, and on the console.
Using portMAX_DELAY, esp_websocket_client_close might wait forever on STOPPED_BIT and we never reach the end of was_deinit_task. Instead, wait for 5 seconds. If esp_websocket_client_close receives STOPPED_BIT within the timeout, it returns ESP_OK. If it doesn't receive STOPPED_BIT within the timeout, it will do the same as esp_websocket_client_stop but also return ESP_OK. Therefore, call esp_websocket_client_stop after esp_websocket_client_close to make sure we actually stop the client.
We currently use the default (WEBSOCKET_RECONNECT_TIMEOUT_MS), which is not guaranteed to always be 10000. This is in preparation of reworking the reconnect logic when the client is not connected at the time of sending a message.
In commit 985375c ("was: drop esp_websocket_client_destroy()") we've dropped esp_websocket_client_destroy() in was_deinit_task(), as it appeared to cause crashes. We're still seeing crashes in the WebSocket client, so let's drop the use of esp_websocket_client_destroy() completely. Instead, if the WAS WebSocket client appears to be not connected, wait WAS_RECONNECT_TIMEOUT_MS for it to reconnect, and if after that it's still not connected, inform the user.
We already explicitly handle HTTP 401 and 406 from WIS, but we don't print anything to the screen when we hit e.g. a 403 or 404. Handle these with a generic message printed to the screen, so the user has some indication about what is going on. Also trigger the configured audio response.
On STT error, the pipeline would stop after printing "STT error", and the line would be overwritten with "Thinking".
stintel
force-pushed
the
feature/server_failures
branch
from
f87554c
to
d0bd3e3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.