Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Axios timeouts are not properly handled #2139

Open
benoit74 opened this issue Jan 27, 2025 · 3 comments
Open

Axios timeouts are not properly handled #2139

benoit74 opened this issue Jan 27, 2025 · 3 comments
Assignees
Labels
Milestone

Comments

@benoit74
Copy link
Contributor

See https://axios-http.com/docs/cancellation

From what I read there and what I find in the code, it seems that:

  • Axios by itself only handles timeouts on response-related timeout
  • Axios does not handles other timeouts linked to connection issues, it will only timeout when caller timeout
  • mwoffliner has no logic to handle these connection issues
  • mwoffliner been a "client" application, it has no caller/stack timeouts like a webserver would have, so in case of connection issues it never timeouts

I consider this is a very probable explanation of some issues we've seen around mwoffliner been like frozen / dead

@benoit74 benoit74 added the bug label Jan 27, 2025
@benoit74 benoit74 added this to the 1.14.1 milestone Jan 27, 2025
@benoit74 benoit74 self-assigned this Jan 27, 2025
@kelson42
Copy link
Collaborator

kelson42 commented Jan 27, 2025

@benoit74 Seems a promising way of investigation indeed! AFAIK we are stuck we a relatively old version of Axios.

@Optimus-NP
Copy link

We currently use the Axios API in 9 places across our codebase. Below is a summary of its usage:

File Method Has Catch Block Has Timeout Uses validateStatus
util/dump.ts GET No Yes (60000) Yes
util/misc.ts GET No No No
util/mw-api.ts GET No No No
sanitize-argument.ts GET Yes No No
sanitize-argument.ts GET Yes No No
MediaWiki.ts POST No No No
Downloader.ts GET Yes No No
Downloader.ts GET No No No
Downloader.ts GET No No No
mwoffliner.lib.ts GET Yes No No

To improve reliability and consistency, I plan to implement an Axios wrapper that will:

  • Enforce a timeout for all requests.
  • Support early aborting if an HTTP connection cannot be established.
  • Implement automatic retries for requests that fail due to timeouts.
  • Include validation status checks to prevent retries on 4XX errors.

This wrapper will help standardize API calls across our codebase and improve fault tolerance.

@benoit74
Copy link
Contributor Author

benoit74 commented Feb 3, 2025

Issue is already assigned, I'm already working on it and a PR is even under way (not yet totally ready, but close to).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants