Skip to content

feat(activesync): stream Sync responses to prevent client read timeouts#84

Merged
ralflang merged 7 commits into
FRAMEWORK_6_0from
feat/sync-response-streaming
Jul 18, 2026
Merged

feat(activesync): stream Sync responses to prevent client read timeouts#84
ralflang merged 7 commits into
FRAMEWORK_6_0from
feat/sync-response-streaming

Conversation

@TDannhauer

@TDannhauer TDannhauer commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds opt-in streamed Sync response delivery: WBXML is flushed to the client incrementally (chunked HTTP) instead of buffering the whole response behind a Content-Length header. Implements feat(activesync): stream Sync responses to avoid client read timeouts (Gmail Android) #83, motivated by email sync quite erratic against both Gmail and Nine on Android #77.
  • Covers both directions of a Sync request: per-message flushing during export, and deferred import of client-sent commands with WBXML keep-alive tokens during large up-sync batches (e.g. Gmail's FullDraftsUpSync).
  • Restructures the package documentation by audience: landing-page README.md plus doc/ guides for administrators, integrators, and developers, including a dedicated design document for streaming and equal coverage of all EAS protocol versions (2.5–16.1).

Motivation

Some clients — notably Gmail on Android — abort a Sync connection after ~30 seconds without response body bytes (SocketTimeout), retry with the old sync key, and can end up in a permanently broken sync state (#77). The client timeout is on time to first/next byte, not total request duration, so keeping bytes flowing fixes the class of failures without artificial batch limits.

Two silent periods could exceed that budget:

  1. Export: fetching/encoding a large server→client batch before the buffered response was sent.
  2. Import: applying a large client→server batch while parsing the request, before any response byte existed (observed: ~55 s of silence for a 200-command Drafts up-sync).

Changes

  • Request_Sync: streaming mode flushes after the envelope, after every exported message, and at folder close. Incoming Add/Change/Delete commands are queued during parsing and imported during response output — at the same logical position relative to change detection and sync-key generation — with a keep-alive flushed between imports. Import errors are reported in-protocol (per-command SyncReplies statuses) instead of HTTP 500.
  • Wbxml_Encoder: idempotent WBXML header; new keepAlive() emitting a redundant SWITCH_PAGE to the active code page (semantic no-op for conforming parsers, verified against this package's own decoder).
  • MoreAvailable ordering without buffering: the maxmessagesperresponse count cap is folded into the effective window size up front, so truncation is known before the Commands block starts; the legacy maxresponsetime budget only applies when streaming is off.
  • New config keys under $conf['activesync']['sync']: streaming (default false), maxmessagesperresponse, maxmessagetime, maxrequestduration. Fully reversible: disabling streaming restores the exact buffered behaviour.
  • Documentation: new doc/configuration.md, doc/integration.md, doc/architecture.md, doc/protocol-versions.md, doc/sync-streaming.md; README.md rewritten as a landing page with a doc index.
  • Tests: test/unit/Horde/ActiveSync/Request/SyncStreamingTest.php (keep-alive transparency to the decoder, header idempotence, deferred-import execution and bookkeeping, config plumbing).

Companion changes:

  • horde/rpcHorde_Rpc_ActiveSync streaming path for Cmd=Sync POST only (no output buffer, no zlib, no Content-Length, headers_sent()-guarded error paths): feat(rpc): stream ActiveSync Sync response bodies Rpc#5.
  • horde/horderpc.php config passthrough and conf.xml keys: already merged into FRAMEWORK_6_0.

Expected exception types in new catch blocks: Horde_ActiveSync_Exception (and subtypes) around deferred command import and streamed export; failures are logged and converted to per-command/folder statuses.

Test plan

Flush WBXML to the client after the envelope status, after every
exported message, and at each folder close when the new 'streaming'
sync config is enabled, so clients with hard read timeouts (Gmail
Android, ~30s without body bytes) no longer abort large syncs.

- Encoder::flushOutput() pushes the WBXML stream and the SAPI buffer.
- The count-based maxmessagesperresponse cap is folded into the
  effective window before Commands starts, so MOREAVAILABLE always
  precedes Commands via the window-exceeded path without buffering.
- The legacy maxresponsetime budget and its Commands buffer are
  bypassed while streaming (log notice when both are configured).
- New maxmessagetime / maxrequestduration guards stop a batch between
  messages; unsent changes stay in sync_pending.
- Post-commit abort handling: exporter exceptions mid-stream are
  logged and the WBXML envelope is closed instead of rethrowing to the
  RPC layer (HTTP 500 is impossible after the first body byte).
- README: new "Sync response streaming" section (architecture, config,
  error model, operator notes); doc/todo.md updated.

Requires the companion streaming path in horde/rpc and the config
passthrough in horde/horde (same branch name in both repos).

Refs #83, #77.
maximumwindowsize (client WindowSize override) and the streaming
maxmessagesperresponse cap (#83) both feed the same export-loop bound
via min(). Record in the Horde 6 roadmap that the Sync options /
configuration-builder refactor should collapse them into one batching
policy; they stay separate on FRAMEWORK_6_0 for rollback semantics.
…en streaming

Streaming (phases 1-4) fixed export-side stalls, but a large client
upload batch (e.g. Gmail FullDraftsUpSync with 200 Modify commands)
was still imported to the backend during request parsing - before any
response byte - leaving the wire silent well past hard client read
timeouts (~30s on Gmail Android).

When streaming is enabled, client-sent ADD/MODIFY/REMOVE commands are
now only queued during parsing and imported during response output,
after the flushed envelope status, at the same logical position the
inline imports occupied (before change detection and synckey
generation). Between imports the encoder flushes a WBXML keep-alive
token (redundant SWITCH_PAGE to the active code page, a no-op for
conforming parsers) so response bytes keep flowing for the whole
import phase. Import errors are recorded as per-command reply statuses
instead of being thrown through the streamed body. The non-streaming
flow is unchanged; the shared import logic is extracted into helpers
used by both paths.
Split the monolithic README into a landing page plus doc/ guides for
administrators (configuration.md), integrators (integration.md), and
developers (architecture.md), with cross-cutting references for EAS
protocol versions 2.5-16.1 (protocol-versions.md) and the streamed Sync
delivery design (sync-streaming.md). Document all protocol versions
equally instead of highlighting 16.x only. Add @see pointers from
Request_Sync and Encoder::keepAlive() to the streaming design doc.
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

🔍 CI Results

Overall: ❌ 12/12 lanes failed

TL;DR: ❌ Quality issues: PHPUnit: 54 tests failed; PHPStan: 329 unique errors in 8 lanes; PHP-CS-Fixer: 22 files.

Summary by PHP Version

PHP dev stable
8.0
8.1
8.2
8.3
8.4
8.5

Quality Metrics

  • PHPUnit: 16 failures, 38 errors out of 223 tests in 8 lanes ❌
    Per-test detail in the table below. Raw JUnit XML and JSON are uploaded as workflow artifacts.

    Which tests failed
    Type Test Lanes Message
    ❌ failure Horde\ActiveSync\AppointmentTest::testSetRecurrenceEas16SetsFirstDayOfWeek php8.2-dev, php8.2-stable, php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [PHPUnit\Framework\ExpectationFailedException] Failed asserting that '' matches expected 0.
    ⚠️ error Horde\ActiveSync\FolderUidDeterministicTest::testFolderUidIsDeterministicAcrossDriverInstances php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\FolderUidDeterministicTest::testFolderUidIsDeterministicForRepeatedLookups php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\FolderUidDeterministicTest::testFolderUidUsesPersistedMapWhenPresent php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\Request\FindRequestTest::testMockDriverReturnsSuccessfulEmptyFindResults php8.2-dev, php8.2-stable, php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [InvalidArgumentException] Missing required state object
    ❌ failure Horde\ActiveSync\Request\FindRequestTest::testParseFindRangeHonorsClientWindow php8.2-dev, php8.2-stable, php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [PHPUnit\Framework\ExpectationFailedException] Failed asserting that 100 is identical to 101.
    ⚠️ error Horde\ActiveSync\SyncTimeBudgetTest::testGetSyncConfigDefaultsToEmptyArray php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
    ⚠️ error Horde\ActiveSync\SyncTimeBudgetTest::testGetSyncConfigReturnsConfiguredValues php8.3-dev, php8.3-stable, php8.4-dev, php8.4-stable, php8.5-dev, php8.5-stable [Error] Call to undefined method PHPUnit\Framework\MockObject\MockBuilder::getMockForAbstractClass()
  • PHPStan (advisory): 329 unique errors in 8 lanes ⚠️

  • PHP-CS-Fixer: 22 unique files with issues (of 201 checked) ⚠️

❌ Failed Lanes

php8.0-dev

  • PHPUnit: ❌ Setup failed (No result file found)
  • PHPStan: ❌ Setup failed (No result file found)

php8.0-stable

  • PHPUnit: ❌ Setup failed (Composer install failed in /tmp/horde-ci/lanes/php8.0-stable/ActiveSync: Your requirements could not be resolved to an installable set of packages.)
  • PHPStan: ❌ Setup failed (Composer install failed in /tmp/horde-ci/lanes/php8.0-stable/ActiveSync: Your requirements could not be resolved to an installable set of packages.)

php8.1-dev

  • PHPUnit: ❌ Setup failed (No result file found)
  • PHPStan: ❌ Setup failed (No result file found)

php8.1-stable

  • PHPUnit: ❌ Setup failed (No result file found)
  • PHPStan: ❌ Setup failed (No result file found)

php8.2-dev

  • PHPUnit: 2 failures, 1 error

php8.2-stable

  • PHPUnit: 2 failures, 1 error

php8.3-dev

  • PHPUnit: 2 failures, 6 errors

php8.3-stable

  • PHPUnit: 2 failures, 6 errors

php8.4-dev

  • PHPUnit: 2 failures, 6 errors
  • PHP-CS-Fixer: 22 files with issues

php8.4-stable

  • PHPUnit: 2 failures, 6 errors

php8.5-dev

  • PHPUnit: 2 failures, 6 errors

php8.5-stable

  • PHPUnit: 2 failures, 6 errors

CI powered by horde-componentsView full results

Copilot AI left a comment

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.

Pull request overview

This PR introduces an opt-in streaming mode for Sync responses so clients with strict “time-to-first/next-byte” read timeouts (notably Gmail on Android) keep receiving response bytes while the server is still exporting changes and/or importing large up-sync batches. It also reorganizes documentation into README.md + audience-targeted guides under doc/, and adds unit coverage for the new streaming behavior.

Changes:

  • Add streamed Sync delivery: incremental WBXML flushing during export, plus deferred import of client commands with WBXML keep-alive tokens during long up-sync work.
  • Extend WBXML encoder with idempotent header emission, flushOutput(), and keepAlive().
  • Add/refresh documentation and add unit tests validating streaming mechanics and bookkeeping.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/Horde/ActiveSync/Request/Sync.php Implements streaming mode, deferred imports, keep-alives, and new streaming-related guards/caps.
lib/Horde/ActiveSync/Wbxml/Encoder.php Adds streaming primitives (flushOutput(), keepAlive()) and idempotent header emission.
test/unit/Horde/ActiveSync/Request/SyncStreamingTest.php Adds unit tests for streaming window sizing, header idempotence, keep-alive transparency, and deferred import behavior.
README.md Refactors into a landing page with a documentation index and updated feature highlights.
doc/sync-streaming.md Design document for streaming Sync responses, including error model and operator notes.
doc/configuration.md Admin-facing configuration guide including streaming options and deployment prerequisites.
doc/integration.md Integrator guide for embedding the library and handling streamed vs buffered responses.
doc/architecture.md Developer-oriented internal architecture overview including streaming mechanics.
doc/protocol-versions.md Protocol version negotiation and per-version behavior overview.
doc/todo.md Updates roadmap notes, including streaming status and follow-ups.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/Horde/ActiveSync/Wbxml/Encoder.php
Comment thread lib/Horde/ActiveSync/Request/Sync.php
Encoder::flushOutput() now also flushes an active PHP output buffer
before flush(): the Horde RPC layer removes buffers when streaming,
but output_buffering=On or non-Horde integrators may still have one
active, which would silently hold the streamed bytes back.

Reshape the delete branch of _importRemoves() to the same
['results' => ..., 'missing' => ...] structure importMessageMove()
returns, replacing the confusing legacy self-assignment
($results['results'] = $results) that was moved here from the
inline import path.
Round-3 reporter testing (issue #77) showed Gmail Android receiving a
streamed Sync response without timing out - the transport fix works -
but then retrying with the old synckey, i.e. discarding the response.
Prime suspect is the volume of WBXML keep-alive tokens: one redundant
SWITCH_PAGE per imported command (200 in the failing request) may
exceed what Gmail's stricter-than-spec parser tolerates.

Emit keep-alives at most once per interval instead of per command:
new keepaliveinterval sync setting, default 15s - safely below the
~30s client read timeout while reducing 200 tokens to 3 for a 57s
import phase. 0 restores per-command emission. The import summary log
line now reports how many keep-alives were emitted.
The MODIFY import path dropped the conversationid/conversationindex
returned by the backend, unlike the ADD path. For EAS 16 email
collections the exporter skips SYNC_MODIFY replies for items without
attachment or conversation data, so a Drafts up-sync (e.g. Gmail's
FullDraftsUpSync) received a response with no Replies block at all.
Gmail rejects such a response and re-sends the same command batch
indefinitely, never advancing its synckey.

Preserve the conversation data like the ADD path does so every
successfully imported draft modify gets its reply. Affects both the
streaming (deferred) and legacy inline import paths, which share
_importSyncCommand().
@ralflang
ralflang merged commit 1cfd71a into FRAMEWORK_6_0 Jul 18, 2026
1 check failed
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.

3 participants