Skip to content

Comments

fix: add error handling for speed test failures#134

Closed
Aaradhya-07 wants to merge 1 commit intom-lab:sandbox-roberto-rewritefrom
Aaradhya-07:fix/add-error-handling-speedtest
Closed

fix: add error handling for speed test failures#134
Aaradhya-07 wants to merge 1 commit intom-lab:sandbox-roberto-rewritefrom
Aaradhya-07:fix/add-error-handling-speedtest

Conversation

@Aaradhya-07
Copy link

The startTest() method in app.js awaits runNdt7() and runMSAK() without any error handling. If either test fails — due to a network error, server timeout, or any thrown exception — the UI remains stuck in the "Testing" state with no way for the user to retry. This PR wraps the test execution in a try/catch/finally block: on success, results display as before; on failure, the user sees a "Test failed. Please try again." message; and in both cases, the finally block resets testRunning and updates the UI so the start button becomes clickable again.

@robertodauria
Copy link
Contributor

Thanks for the contribution. The idea of handling test failures gracefully is sound - if runNdt7() or runMSAK() throws, the UI does get stuck. However, there are a few issues with the implementation:

  • The error UX needs more thought: what happens to the gauge? Are partial results from the first test still displayed? Should there be an explicit retry affordance?
  • runPT() is inside the try block but not awaited, so its errors won't be caught.

This also targets sandbox-roberto-rewrite, which is an active rewrite tracked in #83. Once that lands on main, it would be a good time to revisit this with a more complete approach to error states. Feel free to re-open against main at that point, ideally after coordinating with a maintainer on the UX.

On a broader note: we've been receiving a large volume of unsolicited, sometimes AI-generated PRs with no prior discussion or contact, and as a small team this is not sustainable. We are updating our contribution requirements. Going forward, please coordinate with a maintainer before opening PRs or issues. See https://github.com/m-lab/gsoc#2-engage-with-us for details.

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.

2 participants