Skip to content

fix: add request timeouts to prevent indefinite hangs (#239)#240

Merged
bjornars merged 2 commits into
novem-code:mainfrom
bjornars:bsn/fix-239
Jun 17, 2026
Merged

fix: add request timeouts to prevent indefinite hangs (#239)#240
bjornars merged 2 commits into
novem-code:mainfrom
bjornars:bsn/fix-239

Conversation

@bjornars

Copy link
Copy Markdown
Contributor

Summary

  • Set a default (10s connect, 2min read) timeout on every requests.Session via functools.partial on session.request — one line in NovemAPI.__init__ covers all subclasses with no per-call-site changes
  • job.run() overrides to (30s connect, 30min read) since job execution can legitimately take up to 30 minutes
  • Applied the same fix to NovemGQL which manages its own session independently and was not covered by the NovemAPI change

How it works

self._session.request = functools.partial(self._session.request, timeout=(10, 120))

functools.partial keyword arguments are overridable at call time, so job.run() can pass timeout=(30, 1800) and it takes precedence over the default.

Test plan

  • test_session_default_timeout — mocks session.send and verifies the default (10, 120) timeout flows through the full chain (functools.partialsession.requestsession.send) for an ordinary API call
  • test_job_run_no_files_uses_job_timeout — same approach, verifies (30, 1800) for the empty-JSON branch of run()
  • test_job_run_with_files_uses_job_timeout — same for the multipart-upload branch of run()

Closes #239

bjornars added 2 commits June 17, 2026 11:35
…P connections

Set a default (10s connect, 2min read) timeout on every requests.Session via
functools.partial so all API calls are covered without per-call-site changes.
job.run() overrides this with (30s connect, 30min read) since job execution
can legitimately take up to 30 minutes. Applied to both NovemAPI and NovemGQL
which manages its own session independently.

Closes novem-code#239
@bjornars bjornars merged commit 8438233 into novem-code:main Jun 17, 2026
5 checks passed
bjornars added a commit that referenced this pull request Jun 18, 2026
Add a README "Error handling" section covering the two newest changes:

- PR #236: writes now raise NovemException (or a subclass) carrying the
  server message instead of silently printing a placeholder. Documents the
  exception hierarchy (all importable from novem.exceptions) and the create
  PUT 409 no-op for objects that already exist.
- PR #240: requests now time out instead of hanging, with the default
  (10s, 2min) and job.run() (30s, 30min) values and the resulting
  requests.exceptions.Timeout.
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.

requests.Session has no timeout — large uploads can hang indefinitely on half-open TCP connections

2 participants