Skip to content

Add --passstderr to forward STDERR to WebSocket clients as tagged JSON - #464

Merged
joewalnes merged 1 commit into
masterfrom
feature/passstderr
Jul 10, 2026
Merged

Add --passstderr to forward STDERR to WebSocket clients as tagged JSON#464
joewalnes merged 1 commit into
masterfrom
feature/passstderr

Conversation

@joewalnes

Copy link
Copy Markdown
Owner

Summary

Rebase and rework of #459 (by @Formatted) onto current master.

Forwards STDERR to WebSocket clients as tagged JSON, alongside tagged STDOUT, so a client can tell the two apart:

{"stream":"stdout","data":"..."}
{"stream":"stderr","data":"..."}

STDERR is still logged server-side either way, same as without the flag. Addresses #403 (open since 2021).

What changed from the original PR

  • Integrated with the goroutine-leak fix. The tagged stdout/stderr readers now select{} on the output send against Terminate's done signal, same as the plain text/binary readers added by an earlier PR. The original PR's readers predate that fix and would have reintroduced the same class of leak (a reader parked forever on a channel send after the relay stops draining). Verified by temporarily reverting just that part and watching the new regression test fail (3 leaked goroutines), then restoring it.
  • --binary and --passstderr are now mutually exclusive, rejected at startup with a clear error. The original PR's StartReading silently dropped --binary whenever --passstderr was set (it checked passStderr before bin), which would silently corrupt binary output instead of erroring. Tagging arbitrary binary chunks as JSON string data isn't implemented, so refusing the combination is safer than a partial implementation.
  • JSON encoding via encoding/json (a small taggedMessage struct) instead of a hand-rolled escaper, so it can't emit invalid JSON for control characters or non-UTF8 bytes the original escaper didn't handle.
  • Tests: a --binary/--passstderr validation unit test, a goroutine-leak regression test mirroring the process-endpoint one, an integration test asserting the tagged JSON over a real WebSocket connection (and that STDERR still reaches the server log), and a QA plan entry.

Testing

  • go build/go vet/gofmt -l ./staticcheck all clean
  • go test ./... -race passes
  • Verified the leak fix is load-bearing: reverted just the select{}/done-channel part of the tagged readers, confirmed TestTerminateUnblocksParkedReader_PassStderr fails (goroutine leak detected), restored it and confirmed it passes

Closes #459 (superseding it with this rebased and extended version).

🤖 Generated with Claude Code

https://claude.ai/code/session_01M882UWfvyaq5KGvaV37idr


Generated by Claude Code

Rebase and rework of #459 (by @Formatted) onto current master.

Forwards STDERR to WebSocket clients as tagged JSON, alongside tagged
STDOUT, so a client can tell the two apart:
  {"stream":"stdout","data":"..."}
  {"stream":"stderr","data":"..."}
STDERR is still logged server-side either way, same as without the
flag. Addresses #403 (open since 2021).

Changes from the original PR:
- The tagged stdout/stderr readers now integrate with the done-channel
  leak fix from the earlier goroutine-leak PR: each select{}s on the
  output send against Terminate's done signal, same as the plain text
  and binary readers, instead of blocking unconditionally. Verified
  by temporarily reverting just that part and watching the new
  regression test fail (3 leaked goroutines), then restoring it.
- --binary and --passstderr are now mutually exclusive, rejected at
  startup with a clear error. The original PR silently dropped
  --binary whenever --passstderr was set (StartReading branched on
  passStderr before bin), which would corrupt binary output instead
  of erroring - tagging arbitrary binary chunks as JSON string data
  isn't implemented, so refusing the combination is safer than a
  partial implementation.
- JSON encoding now goes through encoding/json (a small taggedMessage
  struct) instead of a hand-rolled escaper, so it can't emit invalid
  JSON for control characters or non-UTF8 bytes the original escaper
  didn't handle.
- Added a --binary/--passstderr validation unit test, a goroutine-leak
  regression test mirroring the process-endpoint one, an integration
  test asserting the tagged JSON over a real WebSocket connection (and
  that STDERR still reaches the server log), and a QA plan entry.

Co-Authored-By: Formatted <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01M882UWfvyaq5KGvaV37idr

@github-actions github-actions 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.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'websocketd Performance'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.15.

Benchmark suite Current: fc9973b Previous: e40012e Ratio
connection_storm_100_p95 78.1 ms 66 ms 1.18
connection_storm_100_avg 53.86 ms 45.46 ms 1.18
connection_storm_10_p95 11.1 ms 8.55 ms 1.30
connection_storm_10_avg 9 ms 7 ms 1.29
echo_throughput_us_per_msg 37.047 µs/msg 30.664 µs/msg 1.21

This comment was automatically generated by workflow using github-action-benchmark.

CC: @joewalnes

@joewalnes
joewalnes merged commit 16d3db0 into master Jul 10, 2026
13 checks passed
@joewalnes
joewalnes deleted the feature/passstderr branch July 10, 2026 01:28
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