Skip to content

fix: configurable flush timeout to stop silent event loss on large sessions - #83

Open
Xore wants to merge 2 commits into
langfuse:mainfrom
Xore:fix/manifest-null-defaults-and-flush-timeout
Open

Xore wants to merge 2 commits into
langfuse:mainfrom
Xore:fix/manifest-null-defaults-and-flush-timeout

Conversation

@Xore

@Xore Xore commented Sep 6, 2026

Copy link
Copy Markdown

Problem

flush_and_shutdown_langfuse_client() hands the turn's events to the SDK on a daemon thread and waits with t.join(5.0). A session with a few hundred transcript rows produces an OTLP payload that takes far longer than 5s to upload, so the join returned, the process exited, and every event of that session was dropped — while the hook exited 0 and logged Processed N turns. Nothing in the log said anything had been abandoned.

Reproduced on self-hosted Langfuse v4 (events_only mode): a 254-event session was lost entirely at the 5s cap and ingests completely at a raised one.

Fix

The wait is configurable via CC_LANGFUSE_FLUSH_TIMEOUT, default 120s, read through _opt() like every other option so a value set in the plugin wizard also reaches the hook. Declared in plugin.json's userConfig and in the README option table.

Unusable values fall back and log instead of breaking the wait. Parsing happens before the function's blanket except Exception — inside it, a malformed value made float() raise, the join was skipped entirely, and the session was dropped: the exact failure the cap exists to bound, reached by trying to configure it. Non-numeric, non-positive, NaN, inf and absurdly large values all fall back to the default and log, the way CC_LANGFUSE_STATE_DIR already handles a bad value.

Giving up is no longer silent. When the flush thread is still alive after the cap, the hook logs it and the README troubleshooting table gains that line. Silence is why the original bug cost whole sessions before anyone noticed — the only visible symptom was the pre-existing Processed N turns … but nothing in Langfuse row.

SessionEnd needed more than a longer join. Claude Code gives its SessionEnd hooks the longest timeout any of them declares, floored at 1.5s and capped at 60s (getSessionEndHookTimeoutMs, read out of the 2.1.263 bundle: Math.max(1500, Math.min(maxDeclaredTimeout * 1000, 60000)), and the hooks run under AbortSignal.timeout of that value). Declaring no timeout meant a 1.5s window, so almost nothing shipped at session end regardless of the join. hooks/hooks.json now declares "timeout": 60 on the SessionEnd entry to claim the whole ceiling. Stop is deliberately left alone — it has no such cap and keeps the 600s command default, so it is where large sessions actually finish flushing, and declaring a timeout there would only lower it.

The stale # Cap flush+shutdown at 5s … comment is gone.

Tests

tests/unit/test_flush_timeout.py — default, explicit value, CLAUDE_PLUGIN_OPTION_ fallback, each unusable value, that the resolved cap is the one passed to join(), that a still-running flush is reported, and a regression test that a malformed value still waits for the flush. Full suite: 238 passed.

claude plugin validate passes on the updated plugin.json.

Tradeoff, explicitly

The old 5s cap traded correctness for responsiveness: an unreachable Langfuse could never delay a turn by more than 5s, at the cost of losing large sessions outright. The default now sits on the other side of that trade. Anyone who prefers the old behaviour sets CC_LANGFUSE_FLUSH_TIMEOUT=5. Happy to lower the default if you would rather ship the conservative one.


Side note for anyone running a patched fork of this plugin: Claude Code 2.1.263 (2026-09-06) added strict plugin-manifest validation — a userConfig entry with "default": null on a typed field now fails validation and the entire plugin is silently skipped, all hooks dead. Upstream's manifest is clean; forks that added fields with null defaults need to drop them. Found via ~/.claude/debug/<session>.txt.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant

CLAassistant commented Sep 6, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

…ssions

flush_and_shutdown_langfuse_client() joined the flush thread with a hard
t.join(5.0) inside a daemon thread. Sessions with 200+ transcript rows
produce OTLP payloads that take longer than 5s to upload, so the entire
session's events were dropped while the hook still exited 0 and logged
"Processed N turns".

The cap is now configurable via CC_LANGFUSE_FLUSH_TIMEOUT and defaults
to 120s.

Verified against self-hosted Langfuse v4 (events_only mode): a 254-event
session was lost entirely with the 5s cap; with the raised cap all events
ingest.
@Xore
Xore force-pushed the fix/manifest-null-defaults-and-flush-timeout branch from eee22f2 to bf971f8 Compare September 6, 2026 10:09
@Xore Xore changed the title fix: manifest validation under Claude Code >= 2.1.263 and configurable flush timeout fix: configurable flush timeout to stop silent event loss on large sessions Sep 6, 2026
Follow-up to the previous commit on this branch, which made the flush cap
configurable but left four holes.

Parsing moved out of the function's blanket `except Exception`. A malformed
CC_LANGFUSE_FLUSH_TIMEOUT made float() raise inside that try, so the join was
skipped entirely and the session was dropped: the exact failure the cap exists
to bound, reached by trying to configure it. Unusable values (non-numeric,
non-positive, NaN, inf, absurdly large) now fall back to the default and log,
matching how CC_LANGFUSE_STATE_DIR handles a bad value.

The variable is read through _opt(), like every other option, so a value set
in the plugin wizard reaches the hook. It is now declared in plugin.json's
userConfig and documented in the README's option table.

Giving up is no longer silent: the hook logs when the flush thread is still
alive after the cap, and the troubleshooting table gains that line. Silence is
what made the original bug cost a whole session before anyone noticed.

SessionEnd needed more than a longer join. Claude Code allows its SessionEnd
hooks the longest `timeout` any of them declares, floored at 1.5s and capped
at 60s (getSessionEndHookTimeoutMs, verified in 2.1.263). Declaring no timeout
meant a 1.5s window, so almost nothing shipped at session end no matter what
the join said; hooks.json now declares 60 on that entry to claim the ceiling.
Stop has no such cap and keeps the 600s command default, so it stays where
large sessions actually finish flushing.

Also: the comment above the function still promised a 5s cap, and the stale
default is gone from it.

Covered by tests/unit/test_flush_timeout.py, including a regression test that
a malformed value still waits for the flush.
@Xore

Xore commented Sep 6, 2026

Copy link
Copy Markdown
Author

Pushed aeec1dd after reviewing my own first commit — it fixed the symptom but left four holes, and one of them was worse than the bug.

1. Configuring the timeout could break the wait entirely. The parse sat inside the function's blanket except Exception: pass, so CC_LANGFUSE_FLUSH_TIMEOUT=abc (or 0, or a stray space) made float() raise, the join() never ran, the process exited immediately and the whole session was dropped — the exact failure the cap exists to bound, triggered by trying to configure it. Parsing now happens before the try, and unusable values fall back to the default and log, the way CC_LANGFUSE_STATE_DIR already does.

2. It bypassed _opt(). Every other knob in the file resolves through _opt(), which falls back to CLAUDE_PLUGIN_OPTION_<NAME>, so as written the variable was env-only and unreachable from /plugin configure. Fixed, plus a userConfig entry in plugin.json and a row in the README option table.

3. SessionEnd was capped far below 120s, and not by anything the plugin controlled. This is the part I had wrong. From the 2.1.263 bundle, getSessionEndHookTimeoutMs() is:

Math.max(1500, Math.min(maxTimeoutDeclaredByAnySessionEndHook * 1000, 60000))

and the SessionEnd hooks then run under AbortSignal.timeout() of that value. With no timeout declared the window is 1.5 seconds, so raising the join changed nothing at session end. hooks/hooks.json now declares "timeout": 60 on the SessionEnd entry to claim the ceiling. Stop is left untouched on purpose: it has no equivalent cap and keeps the documented 600s command default, so declaring a timeout there would only lower it — Stop is where a large session actually finishes flushing.

4. Giving up was silent. The hook now logs when the flush thread is still alive after the cap, and the troubleshooting table gains that line. Silence is the whole reason this cost a session before anyone noticed; the only symptom was the existing Processed N turns … but nothing in Langfuse row.

Also dropped the stale # Cap flush+shutdown at 5s … comment the first commit left contradicting its own code.

Tests in tests/unit/test_flush_timeout.py cover the resolution table, that the resolved value is what reaches join(), that an unfinished flush is reported, and a regression test that a malformed value still waits for the flush. 238 pass. claude plugin validate passes on the updated manifest.

One open question for you: the default now sits on the correctness side of a trade the 5s cap made the other way — an unreachable Langfuse can delay the end of a turn by up to the cap. CC_LANGFUSE_FLUSH_TIMEOUT=5 restores the old behaviour, but if you would rather ship a more conservative default (30s?) say so and I will change it.

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