Make question loading resilient to network failures
This commit is contained in:
@@ -7,6 +7,7 @@ const DEFAULT_QUESTION_API_BASE_URL := "https://s3q.ranaze.com/api/questions"
|
||||
const DEFAULT_RATINGS_API_URL := "https://s3q.ranaze.com/api/ratings/me"
|
||||
const DEFAULT_QUESTION_REPORTS_API_URL := "https://s3q.ranaze.com/api/question-reports"
|
||||
const RETRY_DELAYS := [2, 5, 10, 20, 30]
|
||||
const REQUEST_TIMEOUT_SECONDS := 30.0
|
||||
const IMAGE_QUESTION_SEARCH_LIMIT := 200
|
||||
const ATTEMPT_QUEUE_PATH := "user://pending_question_attempts.json"
|
||||
|
||||
@@ -65,18 +66,22 @@ func _ready() -> void:
|
||||
str(ProjectSettings.get_setting("step3/question_reports_api_url", DEFAULT_QUESTION_REPORTS_API_URL)),
|
||||
"/api/question-reports")
|
||||
request = HTTPRequest.new()
|
||||
request.timeout = REQUEST_TIMEOUT_SECONDS
|
||||
request.request_completed.connect(_on_api_questions_loaded)
|
||||
add_child(request)
|
||||
|
||||
attempt_request = HTTPRequest.new()
|
||||
attempt_request.timeout = REQUEST_TIMEOUT_SECONDS
|
||||
attempt_request.request_completed.connect(_on_first_attempt_recorded)
|
||||
add_child(attempt_request)
|
||||
|
||||
rating_request = HTTPRequest.new()
|
||||
rating_request.timeout = REQUEST_TIMEOUT_SECONDS
|
||||
rating_request.request_completed.connect(_on_ratings_loaded)
|
||||
add_child(rating_request)
|
||||
|
||||
report_request = HTTPRequest.new()
|
||||
report_request.timeout = REQUEST_TIMEOUT_SECONDS
|
||||
report_request.request_completed.connect(_on_question_report_submitted)
|
||||
add_child(report_request)
|
||||
|
||||
@@ -108,6 +113,15 @@ func load_questions_from_api(reset_retry: bool = true, excluded_question_ids: Ar
|
||||
retry_attempt = 0
|
||||
next_retry_seconds = 0
|
||||
retry_timer.stop()
|
||||
if is_loading or request.get_http_client_status() != HTTPClient.STATUS_DISCONNECTED:
|
||||
request.cancel_request()
|
||||
is_loading = false
|
||||
call_deferred("load_questions_from_api", false, pending_excluded_question_ids, pending_require_media)
|
||||
return
|
||||
elif is_loading or request.get_http_client_status() != HTTPClient.STATUS_DISCONNECTED:
|
||||
# A late timer or repeated click must not turn an active request into an
|
||||
# immediate ERR_BUSY failure.
|
||||
return
|
||||
|
||||
var should_load_ratings := load_state == "idle"
|
||||
is_loading = true
|
||||
@@ -130,18 +144,21 @@ func load_questions_from_api(reset_retry: bool = true, excluded_question_ids: Ar
|
||||
}
|
||||
var err := request.request(questions_api_url, headers, HTTPClient.METHOD_POST, JSON.stringify(payload))
|
||||
if err != OK:
|
||||
_set_load_error("Failed to request questions API.")
|
||||
_set_load_error("Unable to connect to the question service.")
|
||||
|
||||
|
||||
func _on_api_questions_loaded(result: int, response_code: int, _headers: PackedStringArray, body: PackedByteArray) -> void:
|
||||
is_loading = false
|
||||
if result != HTTPRequest.RESULT_SUCCESS or response_code < 200 or response_code >= 300:
|
||||
_set_load_error("Questions API request failed with status %d." % response_code)
|
||||
var message := "The connection to the question service was interrupted."
|
||||
if response_code > 0:
|
||||
message = "Questions API request failed with status %d." % response_code
|
||||
_set_load_error(message, _is_transient_question_failure(result, response_code))
|
||||
return
|
||||
|
||||
var parsed: Variant = JSON.parse_string(body.get_string_from_utf8())
|
||||
if typeof(parsed) != TYPE_ARRAY:
|
||||
_set_load_error("Questions API response was not an array.")
|
||||
_set_load_error("Questions API response was not an array.", false)
|
||||
return
|
||||
|
||||
var api_questions: Array = []
|
||||
@@ -151,7 +168,7 @@ func _on_api_questions_loaded(result: int, response_code: int, _headers: PackedS
|
||||
api_questions.append(normalized)
|
||||
|
||||
if api_questions.is_empty():
|
||||
_set_load_error("Questions API returned no usable questions.")
|
||||
_set_load_error("Questions API returned no usable questions.", false)
|
||||
return
|
||||
if pending_require_media and not _contains_question_with_media(api_questions):
|
||||
for question in api_questions:
|
||||
@@ -178,7 +195,10 @@ func retry_now() -> void:
|
||||
retry_attempt = 0
|
||||
next_retry_seconds = 0
|
||||
retry_timer.stop()
|
||||
load_questions_from_api(false, pending_excluded_question_ids)
|
||||
if is_loading or request.get_http_client_status() != HTTPClient.STATUS_DISCONNECTED:
|
||||
request.cancel_request()
|
||||
is_loading = false
|
||||
call_deferred("load_questions_from_api", false, pending_excluded_question_ids, pending_require_media)
|
||||
|
||||
|
||||
func load_next_random_question(excluded_question_ids: Array) -> void:
|
||||
@@ -323,15 +343,22 @@ func _set_load_error(message: String, schedule_retry: bool = true) -> void:
|
||||
is_loading = false
|
||||
last_error = message
|
||||
push_warning(message)
|
||||
questions_load_failed.emit(message)
|
||||
if schedule_retry and not AuthState.access_token.is_empty():
|
||||
_schedule_retry()
|
||||
else:
|
||||
load_state = "failed"
|
||||
next_retry_seconds = 0
|
||||
questions_load_failed.emit(message)
|
||||
questions_changed.emit()
|
||||
|
||||
|
||||
func _is_transient_question_failure(result: int, response_code: int) -> bool:
|
||||
if result != HTTPRequest.RESULT_SUCCESS:
|
||||
return true
|
||||
return response_code == 0 or response_code == 408 or response_code == 425 \
|
||||
or response_code == 429 or response_code >= 500
|
||||
|
||||
|
||||
func _schedule_retry() -> void:
|
||||
var delay_index := retry_attempt
|
||||
if delay_index >= RETRY_DELAYS.size():
|
||||
@@ -344,6 +371,9 @@ func _schedule_retry() -> void:
|
||||
|
||||
func _on_retry_timer_timeout() -> void:
|
||||
next_retry_seconds = 0
|
||||
if is_loading or request.get_http_client_status() != HTTPClient.STATUS_DISCONNECTED:
|
||||
retry_timer.start(1.0)
|
||||
return
|
||||
load_questions_from_api(false)
|
||||
|
||||
|
||||
|
||||
@@ -837,6 +837,11 @@ func _on_random_question_changed() -> void:
|
||||
func _on_random_question_failed(message: String) -> void:
|
||||
if not awaiting_next_question:
|
||||
return
|
||||
if QuestionBankService.load_state == "retrying":
|
||||
next_button.text = "Reconnecting..."
|
||||
next_button.disabled = true
|
||||
next_button.tooltip_text = message
|
||||
return
|
||||
awaiting_next_question = false
|
||||
next_button.text = "Retry"
|
||||
next_button.disabled = false
|
||||
|
||||
@@ -11,6 +11,8 @@ func _initialize() -> void:
|
||||
_test_eliminated_choice_uses_rich_text_strikeout()
|
||||
_test_explanation_highlighting_has_no_status_heading()
|
||||
_test_collapsed_followup_review_expands()
|
||||
_test_question_failure_retry_classification()
|
||||
_test_transient_question_failure_keeps_player_waiting()
|
||||
quit(1 if failures > 0 else 0)
|
||||
|
||||
|
||||
@@ -141,6 +143,32 @@ func _test_collapsed_followup_review_expands() -> void:
|
||||
player.free()
|
||||
|
||||
|
||||
func _test_question_failure_retry_classification() -> void:
|
||||
var service = load("res://scripts/autoload/question_bank_service.gd").new()
|
||||
_assert(service._is_transient_question_failure(HTTPRequest.RESULT_CANT_CONNECT, 0), "connection failures should retry")
|
||||
_assert(service._is_transient_question_failure(HTTPRequest.RESULT_SUCCESS, 429), "rate limits should retry")
|
||||
_assert(service._is_transient_question_failure(HTTPRequest.RESULT_SUCCESS, 503), "server failures should retry")
|
||||
_assert(not service._is_transient_question_failure(HTTPRequest.RESULT_SUCCESS, 400), "invalid requests should not retry")
|
||||
_assert(not service._is_transient_question_failure(HTTPRequest.RESULT_SUCCESS, 401), "authentication failures should not retry")
|
||||
service.free()
|
||||
|
||||
|
||||
func _test_transient_question_failure_keeps_player_waiting() -> void:
|
||||
var question_bank = root.get_node("QuestionBankService")
|
||||
var prior_state: String = question_bank.load_state
|
||||
var player = load("res://scripts/ui/question_player.gd").new()
|
||||
player.next_button = Button.new()
|
||||
player.awaiting_next_question = true
|
||||
question_bank.load_state = "retrying"
|
||||
player._on_random_question_failed("Connection interrupted.")
|
||||
_assert(player.awaiting_next_question, "automatic retries should preserve the pending next-question request")
|
||||
_assert(player.next_button.disabled, "next button should remain disabled while reconnecting")
|
||||
_assert(player.next_button.text == "Reconnecting...", "retry state should be visible to the user")
|
||||
question_bank.load_state = prior_state
|
||||
player.next_button.free()
|
||||
player.free()
|
||||
|
||||
|
||||
func _assert(condition: bool, message: String) -> void:
|
||||
if condition:
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user