Skip to content

refactor: add SSE Builder::on_response hook#537

Merged
beekld merged 7 commits into
mainfrom
beeklimt/SDK-2379
May 29, 2026
Merged

refactor: add SSE Builder::on_response hook#537
beekld merged 7 commits into
mainfrom
beeklimt/SDK-2379

Conversation

@beekld
Copy link
Copy Markdown
Contributor

@beekld beekld commented May 21, 2026

Summary

Adds an on_response hook to sse::Builder, invoked once per (re)connect after the HTTP response headers have been received, before any branching on status. Fires for every response (any status, including redirects and errors); does not fire if the connection failed before any response (TCP error, read timeout).

Prep for upcoming FDv2 work that needs to inspect the X-LD-FD-Fallback response header.

Design notes

cURL backend accumulates header lines into a per-transfer http::response_header<> on RequestContext, because HeaderCallback is invoked once per CRLF-terminated line. The hook fires on the empty terminator and is posted to the client strand.

Test plan

  • gtest_launchdarkly-sse-client — green under both Foxy and cURL backends
  • New ClientTest.OnResponseHookFiresWithCustomHeader

Note

Medium Risk
Touches reconnect/header parsing on both Foxy and cURL SSE paths; cURL header assembly is new logic though covered by existing and new client tests.

Overview
Adds sse::Builder::on_response, a callback that runs once per connect/reconnect right after response headers are parsed and before status-based handling (success, redirect, errors). It receives a http::response_header<> so callers can read headers such as X-LD-FD-Fallback without waiting for the SSE body.

Foxy backend invokes the hook synchronously on the client executor when header parsing completes. cURL backend rebuilds the prior Content-Type-only header sniffing into full line-by-line header assembly (status line, OWS trimming, duplicate headers via insert(), redirect chains), emits the hook on the blank line after headers, and posts it to the same executor as other callbacks. Header accumulator state is reset on each new transfer.

Includes ClientTest.OnResponseHookFiresWithCustomHeader for status and custom header visibility.

Reviewed by Cursor Bugbot for commit 733ec51. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread libs/server-sent-events/src/curl_client.cpp
Comment thread libs/server-sent-events/src/curl_client.cpp Outdated
Comment thread libs/server-sent-events/src/curl_client.cpp Outdated
Comment thread libs/server-sent-events/src/curl_client.hpp
Comment thread libs/server-sent-events/src/curl_client.cpp
Comment thread libs/server-sent-events/src/curl_client.cpp Outdated
Comment thread libs/server-sent-events/src/curl_client.cpp
Comment thread libs/server-sent-events/src/curl_client.cpp
Comment thread libs/server-sent-events/src/curl_client.cpp Outdated
Comment thread libs/server-sent-events/src/curl_client.cpp Outdated
Comment thread libs/server-sent-events/src/curl_client.hpp
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03833f5. Configure here.

Comment thread libs/server-sent-events/src/curl_client.cpp
beekld and others added 6 commits May 28, 2026 16:19
Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
@beekld beekld force-pushed the beeklimt/SDK-2379 branch from a404f05 to 2c40df6 Compare May 28, 2026 23:21
}
// insert() preserves duplicate-name headers (Set-Cookie, Via, …);
// set() would collapse them and diverge from the Foxy backend.
context->current_response.insert(std::string(name), std::string(value));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one could still throw for invalid header names.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@beekld beekld merged commit ce27c1a into main May 29, 2026
47 checks passed
@beekld beekld deleted the beeklimt/SDK-2379 branch May 29, 2026 21:16
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