Skip to content

Bugfix server crash #201

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.

Sign up for GitHub

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

Merged
ryansurf merged 3 commits into from
Jun 2, 2026
Merged

Bugfix server crash #201

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
867a161
500 error fix
ryansurf Jun 2, 2026
4e40397
version update
ryansurf Jun 2, 2026
1e61d80
stupid linter fix
ryansurf Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "cli-surf"
version = "2.5.0"
version = "2.5.1"
description = "Command-line surf report tool"
license = "MIT"
authors = ["ryansurf <your@email.com>"] # TODO: email
Expand Down
6 changes: 6 additions & 0 deletions src/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ def get_uv(

# Current values. The order of variables needs to be the same as requested.
current = response.Current()
if current is None:
return "No data"
Comment on lines +109 to +110

@sourcery-ai sourcery-ai Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Returning a string when current is None can make the get_uv return type inconsistent and brittle for callers.

This change makes callers handle both numeric and string return values, which is easy to misuse (e.g. arithmetic or formatting expecting a number). Prefer a numeric-compatible sentinel (e.g. None, math.nan, or a reserved numeric code) or handle the "no data" case at a higher layer so this function always returns a consistent type.

current_uv_index = round(current.Variables(0).Value(), decimal)

return current_uv_index
Expand Down Expand Up @@ -206,6 +208,8 @@ def ocean_information(

# Current values. The order of variables needs to be the same as requested.
current = response.Current()
if current is None:
return [0, 0, 0, 0]
current_wave_height = round(current.Variables(0).Value(), decimal)
current_wave_direction = round(current.Variables(1).Value(), decimal)
current_wave_period = round(current.Variables(2).Value(), decimal)
Expand Down Expand Up @@ -309,6 +313,8 @@ def current_wind_temp(

# Current values. The order of variables needs to be the same as requested.
current = response.Current()
if current is None:
return [0, 0, 0]
current_temperature = round(current.Variables(0).Value(), decimal)
current_wind_speed = round(current.Variables(1).Value(), decimal)
current_wind_direction = round(current.Variables(2).Value(), decimal)
Expand Down
36 changes: 28 additions & 8 deletions src/open_meteo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,38 @@
import sqlite3
import threading

import openmeteo_requests
import requests_cache
from retry_requests import retry

_CACHE_PATH = "/tmp/.cache"
_thread_local = threading.local()


def _create_client() -> openmeteo_requests.Client:
"""Creates a cached, retry-enabled Open-Meteo API client."""
def _build_client() -> openmeteo_requests.Client:
backend = requests_cache.SQLiteCache(
"/tmp/.cache", use_memory=False, wal=True
_CACHE_PATH, use_memory=False, wal=True
)
cache_session = requests_cache.CachedSession(
backend=backend, expire_after=3600
session = requests_cache.CachedSession(backend=backend, expire_after=3600)
return openmeteo_requests.Client(
session=retry(session, retries=5, backoff_factor=0.2)
)
retry_session = retry(cache_session, retries=5, backoff_factor=0.2)
return openmeteo_requests.Client(session=retry_session)


openmeteo_client = _create_client()
def _get_thread_client() -> openmeteo_requests.Client:
if not hasattr(_thread_local, "client"):
_thread_local.client = _build_client()

@sourcery-ai sourcery-ai Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Catching all sqlite3.DatabaseError and blindly rebuilding the client risks masking persistent cache/DB issues.

Rebuilding the client on any sqlite3.DatabaseError with the same cache path could just hit the same failure again if the DB is corrupted or the filesystem is unhealthy. Consider catching a narrower exception (e.g. OperationalError) and/or adding logic to clear/recreate the cache file, cap the number of rebuild attempts, or ultimately propagate the error so callers can handle a persistent failure appropriately.

return _thread_local.client
Comment on lines +22 to +25

@sourcery-ai sourcery-ai Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Thread-local client recreation on error may leave underlying sessions/connections unclosed.

When _thread_local.client is replaced after a DatabaseError, the previous client (and its session/cache backend) is discarded without explicit cleanup. If openmeteo_requests.Client/requests_cache hold SQLite connections or file descriptors, this can leak resources under repeated errors. Consider a helper that calls a close()/context-exit on the old client (if available) before assigning a new one.

Suggested implementation:

def _set_thread_client(client: openmeteo_requests.Client) -> openmeteo_requests.Client:
    """Set the thread-local client, closing any existing client if it exposes a close() method.

    This helps avoid leaking resources (e.g. SQLite connections/file descriptors) when
    the thread-local client is recreated after errors.
    """
    old_client = getattr(_thread_local, "client", None)

    if old_client is not None:
        # Best-effort cleanup of the previous client; ignore cleanup errors.
        close = getattr(old_client, "close", None)
        if callable(close):
            try:
                close()
            except Exception:
                # Intentionally ignore to avoid masking the original error path
                pass

    _thread_local.client = client
    return client


def _get_thread_client() -> openmeteo_requests.Client:
    if not hasattr(_thread_local, "client"):
        # First-time initialization for this thread; no previous client to clean up.
        return _set_thread_client(_build_client())
    return _thread_local.client

To fully address the resource-leak concern on error-triggered client recreation, any code paths that currently do something like:

_thread_local.client = _build_client()

for recovery (e.g. in a DatabaseError handler) should be updated to instead call:

_set_thread_client(_build_client())

This ensures the old client is explicitly cleaned up before being replaced in all cases, not only during first-time initialization in _get_thread_client().



class _ResilientClient:
def weather_api(self, url, params=None):
client = _get_thread_client()
try:
return client.weather_api(url, params=params)
except sqlite3.DatabaseError:
_thread_local.client = _build_client()
return _thread_local.client.weather_api(url, params=params)


openmeteo_client = _ResilientClient()
Loading