From 05d48648284f9fcb3a60107b178f80ddaf987b1f Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Sat, 26 Sep 2026 17:25:07 +0200 Subject: [PATCH] test: strengthen SDK regression coverage and fix MCP client lifecycle --- .changeset/tidy-mcp-lifecycle.md | 5 + .github/workflows/unit-tests.yml | 23 + CONTRIBUTING.md | 10 + gemfiles/otel.gemfile | 7 + gemfiles/otel.gemfile.lock | 425 ++++++++++++++++++ lib/posthog/client.rb | 4 +- spec/isolated/with_active_support.rb | 2 +- spec/isolated/with_active_support_and_oj.rb | 2 +- spec/isolated/with_oj.rb | 2 +- spec/isolated_spec.rb | 18 + spec/posthog/backoff_policy_spec.rb | 27 +- spec/posthog/client_spec.rb | 83 ++-- spec/posthog/feature_flag_evaluations_spec.rb | 16 +- spec/posthog/feature_flag_spec.rb | 19 +- spec/posthog/feature_flags_async_load_spec.rb | 66 ++- spec/posthog/flag_definition_cache_spec.rb | 13 +- spec/posthog/flags_spec.rb | 23 +- spec/posthog/mcp/client_spec.rb | 14 + spec/posthog/mcp/http_transport_spec.rb | 43 +- spec/posthog/mcp/ids_spec.rb | 8 +- spec/posthog/mcp/instrument_spec.rb | 87 +++- spec/posthog/mcp/sanitization_spec.rb | 12 +- spec/posthog/mcp/session_token_spec.rb | 18 +- spec/posthog/mcp/truncation_spec.rb | 8 +- spec/posthog/message_batch_spec.rb | 19 + .../posthog/rails/exception_mechanism_spec.rb | 3 + spec/posthog/rails/logs/appender_spec.rb | 22 +- spec/posthog/rails/logs/fork_safety_spec.rb | 41 +- spec/posthog/rails/logs/rate_limiter_spec.rb | 9 +- spec/posthog/rails/logs/real_export_spec.rb | 22 +- spec/posthog/rails/logs/setup_spec.rb | 8 +- spec/posthog/rails/railtie_spec.rb | 29 +- spec/posthog/rails/request_context_spec.rb | 24 +- spec/posthog/response_spec.rb | 8 +- spec/posthog/send_worker_spec.rb | 104 +++-- spec/posthog/transport_spec.rb | 41 +- spec/spec_helper.rb | 11 +- 37 files changed, 1055 insertions(+), 221 deletions(-) create mode 100644 .changeset/tidy-mcp-lifecycle.md create mode 100644 gemfiles/otel.gemfile create mode 100644 gemfiles/otel.gemfile.lock create mode 100644 spec/isolated_spec.rb diff --git a/.changeset/tidy-mcp-lifecycle.md b/.changeset/tidy-mcp-lifecycle.md new file mode 100644 index 00000000..0a50a387 --- /dev/null +++ b/.changeset/tidy-mcp-lifecycle.md @@ -0,0 +1,5 @@ +--- +"posthog-ruby": patch +--- + +Fix instance tracking for client subclasses so `PostHog::MCP::Client` can initialize outside test mode and shut down without raising. diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 4c94b7d7..ab10ff97 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -39,6 +39,29 @@ jobs: - name: Run RSpec tests run: bundle exec rspec + logs-integration: + name: Logs integration (Ruby ${{ matrix.ruby-version }}) + runs-on: ubuntu-latest + strategy: + matrix: + ruby-version: [3.3, 3.4] + fail-fast: false + env: + BUNDLE_GEMFILE: gemfiles/otel.gemfile + RAILS_ENV: test + GH_ACTIONS_UNIT_TESTS: 1 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: ruby/setup-ruby@984c0c890880bbf811283d6f09c4607c62d210a4 # v1.323.0 + with: + ruby-version: ${{ matrix.ruby-version }} + bundler: 4.0.13 + bundler-cache: true + - name: Require real OpenTelemetry dependencies + run: bundle exec ruby -e "require 'opentelemetry-sdk'; require 'opentelemetry-logs-sdk'; require 'opentelemetry/exporter/otlp_logs'" + - name: Run RSpec with real logs dependencies + run: bundle exec rspec + rubocop: name: RuboCop runs-on: ubuntu-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ddf5f88d..527eb611 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -39,6 +39,16 @@ bundle exec rubocop bundle exec rake public_api:check ``` +The JSON compatibility specs run in separate Ruby processes as part of the default suite. +The real Rails logs export and fork tests require Ruby 3.3+ and the optional OpenTelemetry bundle: + +```bash +BUNDLE_GEMFILE=gemfiles/otel.gemfile bundle install +BUNDLE_GEMFILE=gemfiles/otel.gemfile bundle exec rspec +``` + +Without that bundle, those two integration tests are reported as pending. CI runs both bundles. + The public API snapshot covers both `posthog-ruby` and `posthog-rails`. If you intentionally change either API, update the snapshot and review the diff: ```bash diff --git a/gemfiles/otel.gemfile b/gemfiles/otel.gemfile new file mode 100644 index 00000000..0eb2f6ed --- /dev/null +++ b/gemfiles/otel.gemfile @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +eval_gemfile '../Gemfile' + +gem 'opentelemetry-exporter-otlp-logs' +gem 'opentelemetry-logs-sdk', '>= 0.6.0' +gem 'opentelemetry-sdk' diff --git a/gemfiles/otel.gemfile.lock b/gemfiles/otel.gemfile.lock new file mode 100644 index 00000000..30228f53 --- /dev/null +++ b/gemfiles/otel.gemfile.lock @@ -0,0 +1,425 @@ +PATH + remote: .. + specs: + posthog-ruby (3.26.0) + concurrent-ruby (~> 1) + +GEM + remote: https://rubygems.org/ + specs: + actionpack (7.2.3.1) + actionview (= 7.2.3.1) + activesupport (= 7.2.3.1) + cgi + nokogiri (>= 1.8.5) + racc + rack (>= 2.2.4, < 3.3) + rack-session (>= 1.0.1) + rack-test (>= 0.6.3) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + useragent (~> 0.16) + actionview (7.2.3.1) + activesupport (= 7.2.3.1) + builder (~> 3.1) + cgi + erubi (~> 1.11) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + activesupport (7.2.3.1) + base64 + benchmark (>= 0.3) + bigdecimal + concurrent-ruby (~> 1.0, >= 1.3.1) + connection_pool (>= 2.2.5) + drb + i18n (>= 1.6, < 2) + logger (>= 1.4.2) + minitest (>= 5.1, < 6) + securerandom (>= 0.3) + tzinfo (~> 2.0, >= 2.0.5) + addressable (2.9.0) + public_suffix (>= 2.0.2, < 8.0) + ast (2.4.3) + base64 (0.3.0) + benchmark (0.5.0) + bigdecimal (4.1.3) + builder (3.3.0) + cgi (0.5.1) + commander (5.0.0) + highline (~> 3.0.0) + concurrent-ruby (1.3.8) + connection_pool (3.0.2) + crack (1.0.1) + bigdecimal + rexml + crass (1.0.7) + date (3.5.1) + diff-lcs (1.6.2) + drb (2.2.3) + erb (4.0.4.1) + cgi (>= 0.3.3) + erubi (1.13.1) + google-protobuf (4.36.2) + bigdecimal + rake (~> 13.3) + google-protobuf (4.36.2-aarch64-linux-gnu) + bigdecimal + rake (~> 13.3) + google-protobuf (4.36.2-aarch64-linux-musl) + bigdecimal + rake (~> 13.3) + google-protobuf (4.36.2-arm64-darwin) + bigdecimal + rake (~> 13.3) + google-protobuf (4.36.2-x86_64-darwin) + bigdecimal + rake (~> 13.3) + google-protobuf (4.36.2-x86_64-linux-gnu) + bigdecimal + rake (~> 13.3) + google-protobuf (4.36.2-x86_64-linux-musl) + bigdecimal + rake (~> 13.3) + googleapis-common-protos-types (1.23.0) + google-protobuf (~> 4.26) + haml (7.2.0) + temple (>= 0.8.2) + thor + tilt + hana (1.3.7) + hashdiff (1.2.1) + highline (3.0.1) + i18n (1.14.8) + concurrent-ruby (~> 1.0) + io-console (0.8.2) + irb (1.18.0) + pp (>= 0.6.0) + prism (>= 1.3.0) + rdoc (>= 4.0.0) + reline (>= 0.4.2) + json (2.21.2) + json_schemer (2.5.0) + bigdecimal + hana (~> 1.3) + regexp_parser (~> 2.0) + simpleidn (~> 0.2) + language_server-protocol (3.17.0.6) + lint_roller (1.1.0) + logger (1.7.0) + loofah (2.25.2) + crass (~> 1.0.2) + nokogiri (>= 1.12.0) + mcp (1.5.1) + json_schemer (>= 2.4) + minitest (5.27.0) + nokogiri (1.19.4-aarch64-linux-gnu) + racc (~> 1.4) + nokogiri (1.19.4-aarch64-linux-musl) + racc (~> 1.4) + nokogiri (1.19.4-arm-linux-gnu) + racc (~> 1.4) + nokogiri (1.19.4-arm-linux-musl) + racc (~> 1.4) + nokogiri (1.19.4-arm64-darwin) + racc (~> 1.4) + nokogiri (1.19.4-x86_64-darwin) + racc (~> 1.4) + nokogiri (1.19.4-x86_64-linux-gnu) + racc (~> 1.4) + nokogiri (1.19.4-x86_64-linux-musl) + racc (~> 1.4) + oj (3.17.6) + bigdecimal (>= 3.0) + ostruct (>= 0.2) + opentelemetry-api (1.11.1) + logger + opentelemetry-common (0.25.1) + opentelemetry-api (~> 1.0) + opentelemetry-exporter-otlp-logs (0.5.1) + google-protobuf (>= 3.18) + googleapis-common-protos-types (~> 1.3) + opentelemetry-api (~> 1.1) + opentelemetry-common (~> 0.20) + opentelemetry-logs-api (~> 0.1) + opentelemetry-logs-sdk (~> 0.1) + opentelemetry-sdk + opentelemetry-semantic_conventions + opentelemetry-logs-api (0.5.0) + opentelemetry-api (~> 1.0) + opentelemetry-logs-sdk (0.6.1) + opentelemetry-api (~> 1.2) + opentelemetry-logs-api (~> 0.1) + opentelemetry-sdk (~> 1.3) + opentelemetry-registry (0.6.3) + opentelemetry-api (~> 1.1) + opentelemetry-sdk (1.13.1) + logger + opentelemetry-api (~> 1.1) + opentelemetry-common (~> 0.20) + opentelemetry-registry (~> 0.2) + opentelemetry-semantic_conventions + opentelemetry-semantic_conventions (1.44.0) + opentelemetry-api (~> 1.0) + ostruct (0.6.3) + parallel (1.28.0) + parser (3.3.12.0) + ast (~> 2.4.1) + racc + pp (0.6.4) + prettyprint + prettier (4.0.4) + syntax_tree (>= 4.0.1) + syntax_tree-haml (>= 2.0.0) + syntax_tree-rbs (>= 0.2.0) + prettier_print (1.2.1) + prettyprint (0.2.0) + prism (1.9.0) + psych (5.4.0) + date + stringio + public_suffix (6.0.2) + racc (1.8.1) + rack (3.2.6) + rack-session (2.1.2) + base64 (>= 0.1.0) + rack (>= 3.0.0) + rack-test (2.2.0) + rack (>= 1.3) + rackup (2.3.1) + rack (>= 3) + rails-dom-testing (2.3.0) + activesupport (>= 5.0.0) + minitest + nokogiri (>= 1.6) + rails-html-sanitizer (1.7.1) + loofah (~> 2.25, >= 2.25.2) + nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) + railties (7.2.3.1) + actionpack (= 7.2.3.1) + activesupport (= 7.2.3.1) + cgi + irb (~> 1.13) + rackup (>= 1.0.0) + rake (>= 12.2) + thor (~> 1.0, >= 1.2.2) + tsort (>= 0.2) + zeitwerk (~> 2.6) + rainbow (3.1.1) + rake (13.4.2) + rbs (4.0.2) + logger + prism (>= 1.6.0) + tsort + rdoc (7.2.0) + erb + psych (>= 4.0.0) + tsort + regexp_parser (2.12.0) + reline (0.6.3) + io-console (~> 0.5) + rexml (3.4.4) + rspec (3.13.2) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-core (3.13.6) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.5) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.8) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-support (3.13.7) + rubocop (1.91.0) + json (>= 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) + parallel (>= 1.10) + parser (>= 3.3.0.2) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.49.0, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.50.0) + parser (>= 3.3.7.2) + prism (~> 1.7) + ruby-progressbar (1.13.0) + securerandom (0.4.1) + simpleidn (0.3.0) + stringio (3.2.0) + syntax_tree (6.3.0) + prettier_print (>= 1.2.0) + syntax_tree-haml (4.0.3) + haml (>= 5.2) + prettier_print (>= 1.2.1) + syntax_tree (>= 6.0.0) + syntax_tree-rbs (1.0.0) + prettier_print + rbs + syntax_tree (>= 2.0.1) + temple (0.10.4) + thor (1.5.0) + tilt (2.7.0) + timecop (0.9.11) + tsort (0.2.0) + tzinfo (2.0.6) + concurrent-ruby (~> 1.0) + unicode-display_width (3.2.0) + unicode-emoji (~> 4.1) + unicode-emoji (4.2.0) + useragent (0.16.11) + webmock (3.26.4) + addressable (>= 2.8.0) + crack (>= 0.3.2) + hashdiff (>= 0.4.0, < 2.0.0) + zeitwerk (2.7.5) + +PLATFORMS + aarch64-linux-gnu + aarch64-linux-musl + arm-linux-gnu + arm-linux-musl + arm64-darwin + x86_64-darwin + x86_64-linux-gnu + x86_64-linux-musl + +DEPENDENCIES + activesupport (~> 7.1) + commander (~> 5.0) + concurrent-ruby + irb + mcp (>= 1.4) + oj (~> 3.17.6) + opentelemetry-exporter-otlp-logs + opentelemetry-logs-sdk (>= 0.6.0) + opentelemetry-sdk + posthog-ruby! + prettier + railties (~> 7.1) + rake (~> 13.4.2) + rspec (~> 3.13) + rubocop (~> 1.91.0) + timecop + tzinfo (~> 2.0) + webmock + +CHECKSUMS + actionpack (7.2.3.1) sha256=b66afe7f937273270cb63f03bde7af7ba850017867766e8848d06d3e12e1e4ca + actionview (7.2.3.1) sha256=de19b86843391762ac24a6287c30fbba11cd475fa4d4b664924d5fb7a2f1ff7c + activesupport (7.2.3.1) sha256=11ebed516a43a0bb47346227a35ebae4d9427465a7c9eb197a03d5c8d283cb34 + addressable (2.9.0) sha256=7fdf6ac3660f7f4e867a0838be3f6cf722ace541dd97767fa42bc6cfa980c7af + ast (2.4.3) sha256=954615157c1d6a382bc27d690d973195e79db7f55e9765ac7c481c60bdb4d383 + base64 (0.3.0) sha256=27337aeabad6ffae05c265c450490628ef3ebd4b67be58257393227588f5a97b + benchmark (0.5.0) sha256=465df122341aedcb81a2a24b4d3bd19b6c67c1530713fd533f3ff034e419236c + bigdecimal (4.1.3) sha256=61ebe1e5e559bdc3cc6f2c0ee7f427321fc838f59611c294356eb04d6e21cf66 + builder (3.3.0) sha256=497918d2f9dca528fdca4b88d84e4ef4387256d984b8154e9d5d3fe5a9c8835f + bundler (4.0.13) sha256=19f08be7f27022cf0b89f27da0b044ae075e8270a9ef44ad248a932614e1ca3b + cgi (0.5.1) sha256=e93fcafc69b8a934fe1e6146121fa35430efa8b4a4047c4893764067036f18e9 + commander (5.0.0) sha256=22ca42866ce5ed1a7f3e9a9c831a23d03b75b3743c7ad0302c3cbcd9eb5f257d + concurrent-ruby (1.3.8) sha256=b2f1be836e968ccc78ccfce277ea79c72a88633f22306782c16ff23fb415d1e1 + connection_pool (3.0.2) sha256=33fff5ba71a12d2aa26cb72b1db8bba2a1a01823559fb01d29eb74c286e62e0a + crack (1.0.1) sha256=ff4a10390cd31d66440b7524eb1841874db86201d5b70032028553130b6d4c7e + crass (1.0.7) sha256=94868719948664c89ddcaf0a37c65048413dfcb1c869470a5f7a7ceb5390b295 + date (3.5.1) sha256=750d06384d7b9c15d562c76291407d89e368dda4d4fff957eb94962d325a0dc0 + diff-lcs (1.6.2) sha256=9ae0d2cba7d4df3075fe8cd8602a8604993efc0dfa934cff568969efb1909962 + drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373 + erb (4.0.4.1) sha256=47e2a70922ba4cec8c23c2304ad7e581665c37a297cb9a22a860162c7727ee24 + erubi (1.13.1) sha256=a082103b0885dbc5ecf1172fede897f9ebdb745a4b97a5e8dc63953db1ee4ad9 + google-protobuf (4.36.2) sha256=2f8daff50ae14aad36ab0ccdcf284065fa71d726c444efe114e6db82ffe62494 + google-protobuf (4.36.2-aarch64-linux-gnu) sha256=c3279ec6eaa59f6ed6276eda2a33cffb4306471183ab79a332e698b8564f5169 + google-protobuf (4.36.2-aarch64-linux-musl) sha256=0772c9dde1e6dce9a6b44d1d35c28c29adee99d9bd6aff5e21840daeb22161ce + google-protobuf (4.36.2-arm64-darwin) sha256=f70d810e6bbab9d8ad9c1d5fd685a21de4c10ad3ad32efb9455e17e5b1286917 + google-protobuf (4.36.2-x86_64-darwin) sha256=8d1bd2a344f7f589703b202ad2ee4cca8ef731b4ced45782ef92d385967384ba + google-protobuf (4.36.2-x86_64-linux-gnu) sha256=d25a820873e423f1d209ae8c672a4a28b019e2429debd0901d26670a0b4e14db + google-protobuf (4.36.2-x86_64-linux-musl) sha256=0fa17686c896da96a60a705e56ddc7d4ff56413f52787c655bd4c9fb33ffe14f + googleapis-common-protos-types (1.23.0) sha256=992e740a523794d9fc5f29a504465d8fc737aaa16c930fe7228e3346860faf0a + haml (7.2.0) sha256=87fd2b71f7feab1724337b090a7d767f5ab2d42f08c974f3ead673f18cfcd55a + hana (1.3.7) sha256=5425db42d651fea08859811c29d20446f16af196308162894db208cac5ce9b0d + hashdiff (1.2.1) sha256=9c079dbc513dfc8833ab59c0c2d8f230fa28499cc5efb4b8dd276cf931457cd1 + highline (3.0.1) sha256=ca18b218fd581b1fae832f89bfeaf2b34d3a93429c44fd4411042ffce286f009 + i18n (1.14.8) sha256=285778639134865c5e0f6269e0b818256017e8cde89993fdfcbfb64d088824a5 + io-console (0.8.2) sha256=d6e3ae7a7cc7574f4b8893b4fca2162e57a825b223a177b7afa236c5ef9814cc + irb (1.18.0) sha256=de9454a0703a54704b9811a5ef31a60c86949fbf4013fcf244fabc7c775248e3 + json (2.21.2) sha256=1f1d3b7cf2b3ba1a69beca0bb6db13d5438b80bff3cd54cdaaa620b9b07c1c6a + json_schemer (2.5.0) sha256=2f01fb4cce721a4e08dd068fc2030cffd0702a7f333f1ea2be6e8991f00ae396 + language_server-protocol (3.17.0.6) sha256=5ef2c0c138f8267e1bc631d3328347d354f96724b0af22f2c79516120443b7f0 + lint_roller (1.1.0) sha256=2c0c845b632a7d172cb849cc90c1bce937a28c5c8ccccb50dfd46a485003cc87 + logger (1.7.0) sha256=196edec7cc44b66cfb40f9755ce11b392f21f7967696af15d274dde7edff0203 + loofah (2.25.2) sha256=2007f746959ac65552456e04b433e83deb22759ab38c838b4445c70e43425918 + mcp (1.5.1) sha256=883ddd0f59e5bf43805e300158c7ecc4b6a559d126d584d41bd241355545bedf + minitest (5.27.0) sha256=2d3b17f8a36fe7801c1adcffdbc38233b938eb0b4966e97a6739055a45fa77d5 + nokogiri (1.19.4-aarch64-linux-gnu) sha256=1269fb644a6de405057a53dd5c762b1209b43ca7424f839454d3dbc677c31a8f + nokogiri (1.19.4-aarch64-linux-musl) sha256=35c65b9ce72b3bb03207bdbe7067915019dc18c1b9b59139684bd6690fdd01af + nokogiri (1.19.4-arm-linux-gnu) sha256=a301313e38bb065d68239e79734bcd6f56fb6efaacebde29e9abf2a4735340ca + nokogiri (1.19.4-arm-linux-musl) sha256=588923c101bcfa78869734d247d25b598674323e7f22474fc468f6e5647311eb + nokogiri (1.19.4-arm64-darwin) sha256=a46db9853286e6597b36ebc6953817d15acf3a299583eb3f89fdc6f91dd63527 + nokogiri (1.19.4-x86_64-darwin) sha256=7fd17057d3e1f00e9954a74b3cd76595d3d4a5ef233b7ed9599047c204f70551 + nokogiri (1.19.4-x86_64-linux-gnu) sha256=379fae440b28915e3f19d752ce2dcf8465ed2b2fbefd2a7ca0dd497bc981a06a + nokogiri (1.19.4-x86_64-linux-musl) sha256=17dfb7c1fa194ae02fbf7c51a7afc8d278045ab3fdacfd86f91d02d7b274470b + oj (3.17.6) sha256=f511257e01a12030c3ceb8023cd2e96fca924ea9c741f57ea366816f1fd8e68c + opentelemetry-api (1.11.1) sha256=d07f9987ea6240b3fd5ed5dd7450f93c58e403e0553e804069c23ae18ba61ff7 + opentelemetry-common (0.25.1) sha256=38fe87c751e4e13fcd47a8290b4ad67bbedf76c4ed89aeced5ea5c620e66bfb1 + opentelemetry-exporter-otlp-logs (0.5.1) sha256=68e7523bfae7ee107da571cc2c19e0a4873d2e7611613a3888a764ed44bcebec + opentelemetry-logs-api (0.5.0) sha256=81d00aaa3e66887e66f6ea6a3c931e3f1e60b11a3c7e547a54725afe1d5fbc93 + opentelemetry-logs-sdk (0.6.1) sha256=7fd1eccc6793948acff93bbea0dd1aa809a8579cb8fcdea8e0a6580a981a21c2 + opentelemetry-registry (0.6.3) sha256=568fa8eaea94394f29f4fce3f0339ab10c3bf6d6c8beed6b3cfee6fd37fcc608 + opentelemetry-sdk (1.13.1) sha256=91c9cc315ac7ef30243549eeff8b27cd0d39bd02bdf9c9ae481ab96a1cec6754 + opentelemetry-semantic_conventions (1.44.0) sha256=b09b8793422f13b01c08f7a6f29b5f511f47f37bcdb57c0513465a0464168513 + ostruct (0.6.3) sha256=95a2ed4a4bd1d190784e666b47b2d3f078e4a9efda2fccf18f84ddc6538ed912 + parallel (1.28.0) sha256=33e6de1484baf2524792d178b0913fc8eb94c628d6cfe45599ad4458c638c970 + parser (3.3.12.0) sha256=21a6d7f755d5a24dfbdc6e6b772e4e879a52e7631a88bc5a3a134606052c9828 + posthog-ruby (3.26.0) + pp (0.6.4) sha256=dfcb0fce700c41456265922884f9fe195d7fbb0674a3578e6c0f69588e82b570 + prettier (4.0.4) sha256=713110c77675de802806b4e09b4b7783e497689db3e8a991f812aa78f4835bc5 + prettier_print (1.2.1) sha256=a72838b5f23facff21f90a5423cdcdda19e4271092b41f4ea7f50b83929e6ff9 + prettyprint (0.2.0) sha256=2bc9e15581a94742064a3cc8b0fb9d45aae3d03a1baa6ef80922627a0766f193 + prism (1.9.0) sha256=7b530c6a9f92c24300014919c9dcbc055bf4cdf51ec30aed099b06cd6674ef85 + psych (5.4.0) sha256=14f72d69a611af663d7d70e4a7b67d9eb1f3ae9f8d916b478961d5a0075ba5b7 + public_suffix (6.0.2) sha256=bfa7cd5108066f8c9602e0d6d4114999a5df5839a63149d3e8b0f9c1d3558394 + racc (1.8.1) sha256=4a7f6929691dbec8b5209a0b373bc2614882b55fc5d2e447a21aaa691303d62f + rack (3.2.6) sha256=5ed78e1f73b2e25679bec7d45ee2d4483cc4146eb1be0264fc4d94cb5ef212c2 + rack-session (2.1.2) sha256=595434f8c0c3473ae7d7ac56ecda6cc6dfd9d37c0b2b5255330aa1576967ffe8 + rack-test (2.2.0) sha256=005a36692c306ac0b4a9350355ee080fd09ddef1148a5f8b2ac636c720f5c463 + rackup (2.3.1) sha256=6c79c26753778e90983761d677a48937ee3192b3ffef6bc963c0950f94688868 + rails-dom-testing (2.3.0) sha256=8acc7953a7b911ca44588bf08737bc16719f431a1cc3091a292bca7317925c1d + rails-html-sanitizer (1.7.1) sha256=e797a7c9b01e567307e317c576b49ab4168017e63eea4dba9ce3cb587e2f22c2 + railties (7.2.3.1) sha256=aea3393ee10243ceedcbeccb45458a0d58b524b6d21bf32eff8b93853baae15a + rainbow (3.1.1) sha256=039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a + rake (13.4.2) sha256=cb825b2bd5f1f8e91ca37bddb4b9aaf345551b4731da62949be002fa89283701 + rbs (4.0.2) sha256=af75671e66cd03434cc546622741ebf83f6197ec4328375805306330bf78ef25 + rdoc (7.2.0) sha256=8650f76cd4009c3b54955eb5d7e3a075c60a57276766ebf36f9085e8c9f23192 + regexp_parser (2.12.0) sha256=35a916a1d63190ab5c9009457136ae5f3c0c7512d60291d0d1378ba18ce08ebb + reline (0.6.3) sha256=1198b04973565b36ec0f11542ab3f5cfeeec34823f4e54cebde90968092b1835 + rexml (3.4.4) sha256=19e0a2c3425dfbf2d4fc1189747bdb2f849b6c5e74180401b15734bc97b5d142 + rspec (3.13.2) sha256=206284a08ad798e61f86d7ca3e376718d52c0bc944626b2349266f239f820587 + rspec-core (3.13.6) sha256=a8823c6411667b60a8bca135364351dda34cd55e44ff94c4be4633b37d828b2d + rspec-expectations (3.13.5) sha256=33a4d3a1d95060aea4c94e9f237030a8f9eae5615e9bd85718fe3a09e4b58836 + rspec-mocks (3.13.8) sha256=086ad3d3d17533f4237643de0b5c42f04b66348c28bf6b9c2d3f4a3b01af1d47 + rspec-support (3.13.7) sha256=0640e5570872aafefd79867901deeeeb40b0c9875a36b983d85f54fb7381c47c + rubocop (1.91.0) sha256=9c82b7bf391c5d7e3798c5b9996e22a1fe3bd7468e351dfdeb96140c058296d0 + rubocop-ast (1.50.0) sha256=b9ca88300da0803ee222ad20cdb30494c0a784eed06fdc35d254b06d662788db + ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33 + securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1 + simpleidn (0.3.0) sha256=12ca730bed2f3db04d11e9bfd1bca3e11fb37f55b21eb2e9793fb5814bf54d03 + stringio (3.2.0) sha256=c37cb2e58b4ffbd33fe5cd948c05934af997b36e0b6ca6fdf43afa234cf222e1 + syntax_tree (6.3.0) sha256=56e25a9692c798ec94c5442fe94c5e94af76bef91edc8bb02052cbdecf35f13d + syntax_tree-haml (4.0.3) sha256=dcf676677dc28b0f4b908dd414c0e75721fbd6261a2d0f07b378246059196359 + syntax_tree-rbs (1.0.0) sha256=596728cb588a78a197f5b0c0d09e58a00bad1d9856dc3dfaa065199f521436bd + temple (0.10.4) sha256=b7a1e94b6f09038ab0b6e4fe0126996055da2c38bec53a8a336f075748fff72c + thor (1.5.0) sha256=e3a9e55fe857e44859ce104a84675ab6e8cd59c650a49106a05f55f136425e73 + tilt (2.7.0) sha256=0d5b9ba69f6a36490c64b0eee9f6e9aad517e20dcc848800a06eb116f08c6ab3 + timecop (0.9.11) sha256=41284dc6e5041f2184f781ace766f942108c842f8d8c1386a26e6343decc7542 + tsort (0.2.0) sha256=9650a793f6859a43b6641671278f79cfead60ac714148aabe4e3f0060480089f + tzinfo (2.0.6) sha256=8daf828cc77bcf7d63b0e3bdb6caa47e2272dcfaf4fbfe46f8c3a9df087a829b + unicode-display_width (3.2.0) sha256=0cdd96b5681a5949cdbc2c55e7b420facae74c4aaf9a9815eee1087cb1853c42 + unicode-emoji (4.2.0) sha256=519e69150f75652e40bf736106cfbc8f0f73aa3fb6a65afe62fefa7f80b0f80f + useragent (0.16.11) sha256=700e6413ad4bb954bb63547fa098dddf7b0ebe75b40cc6f93b8d54255b173844 + webmock (3.26.4) sha256=8d8da206d217ebe6968cfb09c77f4533c23074e1432bad865f3994eacbaad50d + zeitwerk (2.7.5) sha256=d8da92128c09ea6ec62c949011b00ed4a20242b255293dd66bf41545398f73dd + +BUNDLED WITH + 4.0.13 diff --git a/lib/posthog/client.rb b/lib/posthog/client.rb index 73e21b7c..f8e759e5 100644 --- a/lib/posthog/client.rb +++ b/lib/posthog/client.rb @@ -187,7 +187,7 @@ def initialize(opts = {}) # Warn when multiple clients are created with the same API key (can cause dropped events) unless @disabled || opts[:test_mode] || opts[:disable_singleton_warning] - previous_count = self.class._increment_instance_count(@api_key) + previous_count = PostHog::Client._increment_instance_count(@api_key) if previous_count >= 1 logger.warn( 'Multiple PostHog client instances detected for the same API key. ' \ @@ -947,7 +947,7 @@ def shutdown(timeout: nil) flushed = false begin - self.class._decrement_instance_count(@api_key) unless @disabled + PostHog::Client._decrement_instance_count(@api_key) unless @disabled @feature_flags_poller&.shutdown_poller flushed = if @sync_mode diff --git a/spec/isolated/with_active_support.rb b/spec/isolated/with_active_support.rb index cfd9f2cb..17a5be3e 100644 --- a/spec/isolated/with_active_support.rb +++ b/spec/isolated/with_active_support.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'posthog' require 'isolated/json_example' describe 'with active_support' do diff --git a/spec/isolated/with_active_support_and_oj.rb b/spec/isolated/with_active_support_and_oj.rb index 3dda21fe..2b0f74d0 100644 --- a/spec/isolated/with_active_support_and_oj.rb +++ b/spec/isolated/with_active_support_and_oj.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'posthog' require 'isolated/json_example' describe 'with active_support and oj' do diff --git a/spec/isolated/with_oj.rb b/spec/isolated/with_oj.rb index f9eeb3fe..39c064b6 100644 --- a/spec/isolated/with_oj.rb +++ b/spec/isolated/with_oj.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'posthog' require 'isolated/json_example' describe 'with oj' do diff --git a/spec/isolated_spec.rb b/spec/isolated_spec.rb new file mode 100644 index 00000000..f4dd68c8 --- /dev/null +++ b/spec/isolated_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'open3' + +RSpec.describe 'JSON compatibility in isolated processes' do + %w[with_active_support with_oj with_active_support_and_oj].each do |configuration| + it "serializes batches #{configuration}" do + stdout, stderr, status = Open3.capture3( + RbConfig.ruby, '-Ilib', '-Ispec', Gem.bin_path('rspec-core', 'rspec'), + "spec/isolated/#{configuration}.rb" + ) + + expect(status).to be_success, "#{stdout}\n#{stderr}" + expect(stdout).to include('1 example, 0 failures') + end + end +end diff --git a/spec/posthog/backoff_policy_spec.rb b/spec/posthog/backoff_policy_spec.rb index d1db45de..8d6db2a4 100644 --- a/spec/posthog/backoff_policy_spec.rb +++ b/spec/posthog/backoff_policy_spec.rb @@ -76,15 +76,28 @@ module PostHog ) end - it 'returns exponentially increasing durations' do - expect(subject.next_interval).to be_within(500).of(1000) - expect(subject.next_interval).to be_within(1000).of(2000) - expect(subject.next_interval).to be_within(2000).of(4000) - expect(subject.next_interval).to be_within(4000).of(8000) + it 'returns exponentially increasing durations without jitter' do + allow(subject).to receive(:rand).and_return(0.0) + + expect(Array.new(4) { subject.next_interval }).to eq([1000, 2000, 4000, 8000]) end - it 'caps maximum duration at max_timeout_secs' do - 10.times { subject.next_interval } + [ + [0.25, [1000, 1750, 3500, 7000]], + [0.75, [1375, 2750, 5500, 10_000]] + ].each do |random, expected| + it "applies bounded jitter with random value #{random}" do + allow(subject).to receive(:rand).and_return(random) + + expect(Array.new(4) { subject.next_interval }).to eq(expected) + end + end + + it 'caps maximum duration at max_timeout_ms' do + allow(subject).to receive(:rand).and_return(0.0) + 4.times { subject.next_interval } + + expect(subject.next_interval).to eq(10_000) expect(subject.next_interval).to eq(10_000) end end diff --git a/spec/posthog/client_spec.rb b/spec/posthog/client_spec.rb index 5c64cd47..b992eb74 100644 --- a/spec/posthog/client_spec.rb +++ b/spec/posthog/client_spec.rb @@ -475,18 +475,19 @@ def shutdown expect(client.dequeue_last_message[:properties][:caller_time]).to eq('2024-07-16T13:30:00.123+09:30') end - it 'does not error with the required options' do - expect do - client.capture Queued::CAPTURE - client.dequeue_last_message - end.to_not raise_error - end + [ + { distinct_id: 'user', event: 'checkout', properties: { plan: 'pro' } }, + { 'distinct_id' => 'user', 'event' => 'checkout', 'properties' => { 'plan' => 'pro' } } + ].each do |attributes| + it "enqueues capture with #{attributes.keys.first.class} keys" do + expect(client.capture(attributes)).to be(true) + expect(client.queued_messages).to eq(1) + message = client.dequeue_last_message - it 'does not error when given string keys' do - expect do - client.capture Utils.stringify_keys(Queued::CAPTURE) - client.dequeue_last_message - end.to_not raise_error + expect(message).to include(distinct_id: 'user', event: 'checkout') + expect(message[:properties].transform_keys(&:to_s)).to include('plan' => 'pro') + expect(client.queued_messages).to eq(0) + end end it 'converts time and date properties into iso8601 format' do @@ -1525,18 +1526,19 @@ def shutdown expect { client.identify({}) }.to raise_error(ArgumentError) end - it 'does not error with the required options' do - expect do - client.identify Queued::IDENTIFY - client.dequeue_last_message - end.to_not raise_error - end + [ + { distinct_id: 'user', properties: { plan: 'pro' } }, + { 'distinct_id' => 'user', 'properties' => { 'plan' => 'pro' } } + ].each do |attributes| + it "enqueues identify with #{attributes.keys.first.class} keys" do + expect(client.identify(attributes)).to be(true) + expect(client.queued_messages).to eq(1) + message = client.dequeue_last_message - it 'does not error with the required options as strings' do - expect do - client.identify Utils.stringify_keys(Queued::IDENTIFY) - client.dequeue_last_message - end.to_not raise_error + expect(message).to include(distinct_id: 'user', event: '$identify') + expect(message[:$set].transform_keys(&:to_s)).to eq('plan' => 'pro') + expect(client.queued_messages).to eq(0) + end end it 'converts time and date properties into iso8601 format' do @@ -1568,8 +1570,14 @@ def shutdown end describe '#group_identify' do - it 'errors without group key or group type' do - expect { client.group_identify({}) }.to raise_error(ArgumentError) + [ + [{ group_type: 'organization' }, 'group_key must be given'], + [{ group_key: 'id:5' }, 'group type must be given'] + ].each do |attributes, message| + it "rejects missing group identity: #{message}" do + expect { client.group_identify(attributes) }.to raise_error(ArgumentError, message) + expect(client.queued_messages).to eq(0) + end end it 'identifies group with unique id' do @@ -1584,6 +1592,7 @@ def shutdown ) msg = client.dequeue_last_message + expect(msg[:properties][:$group_key]).to eq('id:5') expect(msg[:distinct_id]).to eq('$organization_id:5') expect(msg[:event]).to eq('$groupidentify') expect(msg[:properties][:$group_type]).to eq('organization') @@ -1627,12 +1636,19 @@ def shutdown expect { client.alias alias: 1234 }.to raise_error(ArgumentError) end - it 'does not error with the required options' do - expect { client.alias ALIAS.dup }.to_not raise_error - end + [ + { distinct_id: 'user', alias: 'anonymous' }, + { 'distinct_id' => 'user', 'alias' => 'anonymous' } + ].each do |attributes| + it "enqueues alias with #{attributes.keys.first.class} keys" do + expect(client.alias(attributes)).to be(true) + expect(client.queued_messages).to eq(1) + message = client.dequeue_last_message - it 'does not error with the required options as strings' do - expect { client.alias Utils.stringify_keys(ALIAS) }.to_not raise_error + expect(message).to include(distinct_id: 'user', event: '$create_alias') + expect(message[:properties]).to include(distinct_id: 'user', alias: 'anonymous') + expect(client.queued_messages).to eq(0) + end end it 'sets distinct_id property' do @@ -1705,13 +1721,14 @@ def run it 'completes when the process forks' do client.identify Queued::IDENTIFY - Process.fork do + pid = Process.fork do client.capture Queued::CAPTURE - client.flush - expect(client.queued_messages).to eq(0) + flushed = client.flush(timeout: 1) + exit!(flushed && client.queued_messages.zero? ? 0 : 1) end - Process.wait + _, status = Process.wait2(pid) + expect(status).to be_success end it 'completes after fork when the async worker has a partial batch' do diff --git a/spec/posthog/feature_flag_evaluations_spec.rb b/spec/posthog/feature_flag_evaluations_spec.rb index d37f6faf..fc93258f 100644 --- a/spec/posthog/feature_flag_evaluations_spec.rb +++ b/spec/posthog/feature_flag_evaluations_spec.rb @@ -161,7 +161,9 @@ def capture_stderr it 'get_flag_payload does not fire an event' do stub_flags(flags_response) snapshot = client.evaluate_flags('user-1') - snapshot.get_flag_payload('variant-flag') + 2.times { expect(snapshot.get_flag_payload('variant-flag')).to eq('key' => 'value') } + expect(snapshot.only_accessed.keys).to eq([]) + expect(WebMock).to have_requested(:post, FLAGS_ENDPOINT).once msgs = drain_messages(client) expect(msgs.any? { |m| m[:event] == '$feature_flag_called' }).to be(false) end @@ -307,6 +309,12 @@ def capture_stderr end expect(WebMock).not_to have_requested(:post, FLAGS_ENDPOINT) expect(warned.any? { |m| m.include?('Both `flags` and `send_feature_flags`') }).to be(true) + event = drain_messages(client).find { |message| message[:event] == 'test-event' } + expect(event[:properties]).to include( + '$feature/variant-flag' => 'variant-value', + '$feature/disabled-flag' => false, + '$active_feature_flags' => %w[boolean-flag variant-flag] + ) end it 'logs and ignores flags: when given a non-snapshot value (no NoMethodError)' do @@ -539,7 +547,7 @@ def capture_stderr stub_flags(flags_response) c = Client.new(api_key: API_KEY, test_mode: true) out = capture_stderr do - 5.times { c.is_feature_enabled('boolean-flag', "user-#{rand(1000)}") } + 5.times { |index| c.is_feature_enabled('boolean-flag', "user-#{index}") } end count = out.scan('`is_feature_enabled` is deprecated').length expect(count).to eq(1) @@ -599,7 +607,9 @@ def capture_stderr stub_request(:get, %r{https://us\.i\.posthog\.com/flags/definitions}) .to_return(status: 200, body: local_definitions.to_json) c = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true) - c.evaluate_flags('user-1', flag_keys: %w[local-flag]) + snapshot = c.evaluate_flags('user-1', flag_keys: %w[local-flag]) + expect(snapshot.keys).to eq(['local-flag']) + expect(snapshot.get_flag('local-flag')).to be(true) expect(WebMock).not_to have_requested(:post, FLAGS_ENDPOINT) end diff --git a/spec/posthog/feature_flag_spec.rb b/spec/posthog/feature_flag_spec.rb index 0993f9b3..8a35615e 100644 --- a/spec/posthog/feature_flag_spec.rb +++ b/spec/posthog/feature_flag_spec.rb @@ -1349,7 +1349,7 @@ module PostHog { 'aggregation_group_type_index' => 0, 'properties' => [], - 'rollout_percentage' => 100 + 'rollout_percentage' => 50 } ] } @@ -1364,11 +1364,22 @@ module PostHog c = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true) + poller = c.instance_variable_get(:@feature_flags_poller) + allow(poller).to receive(:_hash).and_call_original + allow(poller).to receive(:_hash).with('rollout-flag', 'acme').and_return(0.25) + allow(poller).to receive(:_hash).with('rollout-flag', 'other').and_return(0.75) + allow(poller).to receive(:_hash).with('rollout-flag', 'any-distinct-id').and_return(0.75) + expect(c.get_feature_flag( 'rollout-flag', 'any-distinct-id', groups: { 'company' => 'acme' }, group_properties: { 'company' => {} } )).to eq(true) + expect(c.get_feature_flag( + 'rollout-flag', 'any-distinct-id', + groups: { 'company' => 'other' }, + group_properties: { 'company' => {} } + )).to eq(false) assert_not_requested :post, flags_endpoint end end @@ -4853,7 +4864,11 @@ module PostHog c = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true) - expect(c.get_remote_config_payload(encrypted_payload_flag_key)) + expect(c.get_remote_config_payload(encrypted_payload_flag_key)).to eq('super secret payload in plaintext') + expect(WebMock).to have_requested( + :get, + "https://us.i.posthog.com/api/projects/@current/feature_flags/#{encrypted_payload_flag_key}/remote_config?token=testsecret" + ).once assert_not_requested :post, flags_endpoint end diff --git a/spec/posthog/feature_flags_async_load_spec.rb b/spec/posthog/feature_flags_async_load_spec.rb index 56d242e5..e205bf30 100644 --- a/spec/posthog/feature_flags_async_load_spec.rb +++ b/spec/posthog/feature_flags_async_load_spec.rb @@ -16,10 +16,16 @@ module PostHog } end let(:definitions_body) { { flags: [beta_flag_definition] }.to_json } + let(:release) { Queue.new } - # Stop the poller so it doesn't keep hitting (reset) WebMock stubs for the - # rest of the suite. - after { @client&.shutdown } + after do + release.close + if @caller + @caller.kill unless @caller.join(2) + @caller.join + end + @client&.shutdown + end def build_client(**opts) @client = Client.new( @@ -36,42 +42,36 @@ def local_flag_value(client, key = 'beta-feature') client.evaluate_flags('distinct-id', only_evaluate_locally: true).get_flag(key) end - def monotonic_now - Process.clock_gettime(Process::CLOCK_MONOTONIC) - end - describe 'Client.new' do it 'returns immediately, fetching flag definitions asynchronously' do + started = Queue.new stub_request(:get, definitions_endpoint).to_return do - sleep 1 + started << true + release.pop { status: 200, body: definitions_body } end - started = monotonic_now - client = build_client - elapsed = monotonic_now - started - - expect(elapsed).to be < 0.5 - - # no flag definitions yet + @caller = Thread.new { build_client } + eventually { expect(started).not_to be_empty } + expect(@caller.join(1)).to eq(@caller) + client = @caller.value expect(client.feature_flags_loaded?).to be(false) - # flag definitions loaded later, asynchronously + release << true eventually { expect(client.feature_flags_loaded?).to be(true) } expect(local_flag_value(client)).to be(true) end it 'keeps the default synchronous load when the option is not set' do + fetch_thread = nil stub_request(:get, definitions_endpoint).to_return do - sleep 0.3 + fetch_thread = Thread.current { status: 200, body: definitions_body } end - started = monotonic_now client = @client = Client.new(api_key: API_KEY, secret_key: API_KEY, test_mode: true) - elapsed = monotonic_now - started - expect(elapsed).to be >= 0.3 + expect(fetch_thread).to eq(Thread.current) expect(client.feature_flags_loaded?).to be(true) expect(local_flag_value(client)).to be(true) end @@ -82,20 +82,16 @@ def monotonic_now fetches_started = Concurrent::AtomicFixnum.new(0) stub_request(:get, definitions_endpoint).to_return do fetches_started.increment - sleep 1 + release.pop { status: 200, body: definitions_body } end client = build_client eventually { expect(fetches_started.value).to eq(1) } + @caller = Thread.new { Array.new(3) { local_flag_value(client) } } - started = monotonic_now - values = Array.new(3) { local_flag_value(client) } - elapsed = monotonic_now - started - - # the poller's initial load is still sleeping in the stub - expect(values).to eq([nil, nil, nil]) - expect(elapsed).to be < 0.5 + expect(@caller.join(1)).to eq(@caller) + expect(@caller.value).to eq([nil, nil, nil]) expect(fetches_started.value).to eq(1) end end @@ -107,16 +103,19 @@ def monotonic_now if attempts.increment == 1 { status: 500, body: 'error' } else + release.pop { status: 200, body: definitions_body } end end client = build_client(feature_flags_polling_interval: 0.2) - + eventually { expect(attempts.value).to be >= 2 } + expect(client.feature_flags_loaded?).to be(false) expect(local_flag_value(client)).to be_nil + + release.close eventually { expect(client.feature_flags_loaded?).to be(true) } expect(local_flag_value(client)).to be(true) - expect(attempts.value).to be >= 2 end end @@ -126,16 +125,15 @@ def monotonic_now client = build_client eventually { expect(client.feature_flags_loaded?).to be(true) } + fetch_thread = nil stub_request(:get, definitions_endpoint).to_return do - sleep 0.3 + fetch_thread = Thread.current { status: 200, body: { flags: [beta_flag_definition.merge(key: 'newer-feature')] }.to_json } end - started = monotonic_now client.reload_feature_flags - elapsed = monotonic_now - started - expect(elapsed).to be >= 0.3 + expect(fetch_thread).to eq(Thread.current) expect(local_flag_value(client, 'newer-feature')).to be(true) end end diff --git a/spec/posthog/flag_definition_cache_spec.rb b/spec/posthog/flag_definition_cache_spec.rb index 43a2194f..f88abb34 100644 --- a/spec/posthog/flag_definition_cache_spec.rb +++ b/spec/posthog/flag_definition_cache_spec.rb @@ -436,11 +436,18 @@ def get_poller(client) stub_request(:get, local_eval_url) .to_return(status: 200, body: sample_flags_data.to_json) - create_client_with_cache(provider: provider) + client = create_client_with_cache(provider: provider, stub_api: false) + expect(provider.stored_data[:flags].first[:active]).to be(true) - expect(provider.stored_data).not_to be_nil + updated = Marshal.load(Marshal.dump(sample_flags_data)) + updated['flags'].first['active'] = false + stub_request(:get, local_eval_url).to_return(status: 200, body: updated.to_json) + client.reload_feature_flags + + expect(provider.on_received_call_count).to eq(2) expect(provider.stored_data[:flags].length).to eq(2) - expect(provider.stored_data[:flags].first[:key]).to eq('test-flag') + expect(provider.stored_data[:flags].first).to include(key: 'test-flag', active: false) + expect(client.get_feature_flag('test-flag', 'user', only_evaluate_locally: true)).to be(false) end it 'roundtrip: data stored after API fetch can be loaded via JSON serialization' do diff --git a/spec/posthog/flags_spec.rb b/spec/posthog/flags_spec.rb index 85a2ab07..d0f34089 100644 --- a/spec/posthog/flags_spec.rb +++ b/spec/posthog/flags_spec.rb @@ -1198,9 +1198,9 @@ module PostHog expect(locally_evaluated).to be true # Make flag-a inactive - should break the chain - flags = poller.instance_variable_get(:@feature_flags) + flags = Marshal.load(Marshal.dump(poller.instance_variable_get(:@feature_flags))) flags[0][:active] = false - poller.instance_variable_set(:@feature_flags, flags) + stub_feature_flags(flags) result, locally_evaluated = poller.get_feature_flag('flag-d', 'test-user', {}, {}, {}, true) expect(result).to be false @@ -1266,9 +1266,9 @@ module PostHog expect(locally_evaluated).to be true # Email condition satisfied but flag dependency not satisfied (base-flag inactive) - flags = poller.instance_variable_get(:@feature_flags) + flags = Marshal.load(Marshal.dump(poller.instance_variable_get(:@feature_flags))) flags[0][:active] = false - poller.instance_variable_set(:@feature_flags, flags) + stub_feature_flags(flags) result, locally_evaluated = poller.get_feature_flag('mixed-flag', 'test-user-3', {}, { 'email' => 'test@example.com' }, {}, true) @@ -1303,7 +1303,7 @@ module PostHog properties: [ { key: 'base-flag', - operator: 'exact', + operator: 'flag_evaluates_to', value: true, type: 'flag' # No dependency_chain property - should handle gracefully @@ -1316,7 +1316,14 @@ module PostHog } ]) - # Should return [nil, false] when dependency_chain is missing (falls back to remote) + property = { key: 'base-flag', operator: 'flag_evaluates_to', value: true, type: 'flag' } + expect do + poller.evaluate_flag_dependency(property, {}, 'test-user', {}, {}) + end.to raise_error(PostHog::InconclusiveMatchError, /missing required 'dependency_chain'/) + expect(poller.evaluate_flag_dependency( + property.merge(dependency_chain: ['base-flag']), {}, 'test-user', {}, {} + )).to be(true) + result, locally_evaluated = poller.get_feature_flag('missing-chain-flag', 'test-user', {}, {}, {}, true) expect(result).to be_nil expect(locally_evaluated).to be false @@ -1435,9 +1442,9 @@ module PostHog expect(locally_evaluated).to be true # Make one dependency inactive - should fail AND condition - flags = poller.instance_variable_get(:@feature_flags) + flags = Marshal.load(Marshal.dump(poller.instance_variable_get(:@feature_flags))) flags[0][:active] = false # Make flag-a inactive - poller.instance_variable_set(:@feature_flags, flags) + stub_feature_flags(flags) result, locally_evaluated = poller.get_feature_flag('multi-dependency-flag', 'test-user', {}, {}, {}, true) expect(result).to be false diff --git a/spec/posthog/mcp/client_spec.rb b/spec/posthog/mcp/client_spec.rb index a371e3d1..c6589593 100644 --- a/spec/posthog/mcp/client_spec.rb +++ b/spec/posthog/mcp/client_spec.rb @@ -3,6 +3,20 @@ require_relative 'spec_helper' RSpec.describe PostHog::MCP::Client do + describe 'lifecycle' do + [{ test_mode: true }, { sync_mode: true }, {}].each do |options| + it "constructs and shuts down with #{options.inspect}" do + stub_request(:post, 'https://us.i.posthog.com/batch/').to_return(status: 200, body: '{}') + mcp_client = described_class.new({ api_key: 'phc_lifecycle' }.merge(options)) + mcp_client.capture_tool_call('echo', distinct_id: 'user') + + expect(mcp_client.shutdown(timeout: 2)).to be(true) + expect(mcp_client.shutdown(timeout: 2)).to be(true) + expect(mcp_client.capture(event: 'after shutdown', distinct_id: 'user')).to be(false) + end + end + end + let(:client) { described_class.new(api_key: 'phc_test', test_mode: true) } it 'captures tool calls with $lib override, anonymous distinct id and error scalars' do diff --git a/spec/posthog/mcp/http_transport_spec.rb b/spec/posthog/mcp/http_transport_spec.rb index 64d60ff6..aa8878f1 100644 --- a/spec/posthog/mcp/http_transport_spec.rb +++ b/spec/posthog/mcp/http_transport_spec.rb @@ -180,23 +180,34 @@ def attribution(events, name) alice = initialize_as('alice') bob = initialize_as('bob') bob_finished = Queue.new + bob_thread = nil PostHogMcpHttpSpecCaptureTool.before_capture = lambda do # Runs inside Alice's tool body: let Bob's whole request finish before Alice captures. PostHogMcpHttpSpecCaptureTool.before_capture = nil - Thread.new do + bob_thread = Thread.new do call_capture_tool_as('bob', bob, id: 3) bob_finished << true end - bob_finished.pop + eventually { expect(bob_finished).not_to be_empty } + expect(bob_thread.join(2)).to eq(bob_thread) + bob_thread.value end call_capture_tool_as('alice', alice) events = drain_events(client) + expect(events.count { |event| event[:event] == 'in_tool_event' }).to eq(2) + expect(events.count { |event| event[:event] == '$mcp_tool_call' }).to eq(2) expect(attribution(events, 'in_tool_event')).to eq( 'alice' => PostHog::MCP.decode_session_id(alice).session_id, 'bob' => PostHog::MCP.decode_session_id(bob).session_id ) expect(attribution(events, 'in_tool_event')).to eq(attribution(events, '$mcp_tool_call')) + ensure + if bob_thread + bob_thread.kill unless bob_thread.join(2) + bob_thread.join + end + PostHogMcpHttpSpecCaptureTool.before_capture = nil end end end @@ -273,10 +284,11 @@ def dispatching_app(status: 200, body: nil) expect(headers['mcp-session-id']).to be_nil expect(env['posthog_mcp.session']).to be_nil - # A JSON-RPC error rides on a 200, and nothing below minted a token for it. - rejected = JSON.generate({ jsonrpc: '2.0', id: 1, error: { code: -32_602, message: 'Unsupported protocol version' } }) - env = env_for(initialize_json) - _, headers, = described_class.new(dispatching_app(body: rejected)).call(env) + rejected = JSON.parse(initialize_json) + rejected['params']['clientInfo'] = nil + env = env_for(JSON.generate(rejected)) + _, headers, body = described_class.new(dispatching_app).call(env) + expect(JSON.parse(body.first).dig('error', 'code')).to eq(-32_602) expect(headers['mcp-session-id']).to be_nil expect(env['posthog_mcp.session']).to be_nil end @@ -321,17 +333,32 @@ def dispatching_app(status: 200, body: nil) expect(env['posthog_mcp.session']).to be_nil end + it 'preserves an application-owned response session header after minting' do + app = lambda do |env| + env['posthog_mcp.mint'].call(client_name: 'c', protocol_version: '2025-06-18') + [200, { 'Mcp-Session-Id' => 'application-session' }, ['{}']] + end + + _, headers, = described_class.new(app).call(env_for(initialize_json)) + + expect(headers).to eq('Mcp-Session-Id' => 'application-session') + end + it 'attaches the token to a streaming body and never clobbers a replayed header' do PostHog::MCP.instrument(server, client) streaming = Class.new do def initialize(json) = @json = json def each = yield("data: #{@json}\n\n") end + stream = nil sse = lambda do |_env| - [200, { 'content-type' => 'text/event-stream' }, streaming.new(server.handle_json(initialize_json))] + stream = streaming.new(server.handle_json(initialize_json)) + expect(stream).not_to receive(:each) + [200, { 'content-type' => 'text/event-stream' }, stream] end - _, headers, = described_class.new(sse).call(env_for(initialize_json)) + _, headers, body = described_class.new(sse).call(env_for(initialize_json)) expect(headers['mcp-session-id']).not_to be_nil + expect(body).to equal(stream) token = PostHog::MCP.encode_session_id(session_id: 'ses_replayed') replay = env_for('{}', 'mcp-session-id' => token) diff --git a/spec/posthog/mcp/ids_spec.rb b/spec/posthog/mcp/ids_spec.rb index 96028954..55c7ea50 100644 --- a/spec/posthog/mcp/ids_spec.rb +++ b/spec/posthog/mcp/ids_spec.rb @@ -27,9 +27,15 @@ end it 'is time-ordered and unique' do + allow(Process).to receive(:clock_gettime).and_call_original + allow(Process).to receive(:clock_gettime).with(Process::CLOCK_REALTIME, + :millisecond).and_return(1_700_000_000_000) first = Array.new(25) { described_class.uuid_v7 } - sleep 0.005 + allow(Process).to receive(:clock_gettime).with(Process::CLOCK_REALTIME, + :millisecond).and_return(1_700_000_000_001) second = Array.new(25) { described_class.uuid_v7 } + expect(first.map { |uuid| uuid.delete('-')[0, 12].to_i(16) }.uniq).to eq([1_700_000_000_000]) + expect(second.map { |uuid| uuid.delete('-')[0, 12].to_i(16) }.uniq).to eq([1_700_000_000_001]) expect((first + second).uniq.length).to eq(50) expect(first.max).to be < second.min end diff --git a/spec/posthog/mcp/instrument_spec.rb b/spec/posthog/mcp/instrument_spec.rb index 96e211cc..66561aad 100644 --- a/spec/posthog/mcp/instrument_spec.rb +++ b/spec/posthog/mcp/instrument_spec.rb @@ -126,8 +126,14 @@ def initialize_request(id = 1, version = '2025-06-18') expect(Kernel).to receive(:warn).with(a_string_including('experimental')).once handle = described_class.instrument(server, client) expect(handle).to be_a(PostHog::MCP::Analytics) - expect(described_class.instrument(server, client)).to be_a(PostHog::MCP::Analytics) - expect(described_class.tracking_data(server).server_name).to eq('spec-server') + data = described_class.tracking_data(server) + other_client = new_test_client + expect(described_class.instrument(server, other_client)).to be_a(PostHog::MCP::Analytics) + expect(described_class.tracking_data(server)).to equal(data) + expect(data.server_name).to eq('spec-server') + server.handle(initialize_request) + expect(drain_events(client).map { |event| event[:event] }).to eq(['$mcp_initialize']) + expect(drain_events(other_client)).to be_empty end it 'returns a no-op handle for unsupported servers and when no client is available' do @@ -216,7 +222,33 @@ def initialize_request(id = 1, version = '2025-06-18') expect(events.map { |e| e[:properties]['$session_id'] }.uniq.length).to eq(1) end - it 'strips the injected context before the tool sees it but keeps a tool-owned context' do + it 'strips injected arguments before calling a strict keyword tool' do + strict_tool = Class.new(MCP::Tool) do + tool_name 'strict_echo' + input_schema(properties: { message: { type: 'string' } }, required: ['message']) + define_singleton_method(:call) do |message:| + MCP::Tool::Response.new([{ type: 'text', text: message }]) + end + end + strict_server = MCP::Server.new(name: 'strict', tools: [strict_tool]) + described_class.instrument(strict_server, client) + arguments = { message: 'hi', context: 'testing', llm_model: 'test-model', + conversation_id: '0198d3a7-1111-7222-8333-444455556666' } + + 2.times do |index| + strict_server.handle(rpc(10, 'tools/list')) if index.positive? + response = strict_server.handle(rpc(index, 'tools/call', { name: 'strict_echo', arguments: arguments.dup })) + expect(response.dig(:result, :content)).to eq([{ type: 'text', text: 'hi' }]) + end + calls = events_named(drain_events(client), '$mcp_tool_call') + expect(calls.length).to eq(2) + calls.each do |call| + expect(call[:properties].dig('$mcp_parameters', 'request', 'params', 'arguments')).to eq('message' => 'hi') + expect(call[:properties]['$mcp_intent']).to eq('testing') + end + end + + it 'keeps a tool-owned context' do described_class.instrument(server, client, capture_model: false, enable_conversation_id: false) result = server.handle(rpc(1, 'tools/call', { name: 'owns_context', arguments: { context: 'mine' } })) expect(result[:result][:content][0][:text]).to eq('ctx=mine') @@ -373,6 +405,16 @@ def initialize_request(id = 1, version = '2025-06-18') expect(events[1][:properties]).not_to have_key('$mcp_intent') end + it 'drops telemetry when before_send raises without breaking the tool response' do + described_class.instrument(server, client, enable_conversation_id: false, + before_send: ->(_payload) { raise 'filter failed' }) + + response = server.handle(rpc(1, 'tools/call', { name: 'echo', arguments: { message: 'hi' } })) + + expect(response.dig(:result, :content)).to eq([{ type: 'text', text: 'Echo: hi' }]) + expect(drain_events(client)).to be_empty + end + it 'attributes a request to the identity it resolved, not to one a later request installed' do PostHogMcpSpecParkedTool.entered = Queue.new PostHogMcpSpecParkedTool.release = Queue.new @@ -389,12 +431,13 @@ def initialize_request(id = 1, version = '2025-06-18') ) alice = Thread.new { shared.handle(rpc(1, 'tools/call', { name: 'parked', arguments: { user: 'alice' } })) } - PostHogMcpSpecParkedTool.entered.pop + eventually { expect(PostHogMcpSpecParkedTool.entered).not_to be_empty } # Bob's request lands whole - identify included - while Alice is parked in # her tool body. Over stdio both share the server-wide session, so Bob's # identify re-points that session's cache entry at Bob. shared.handle(rpc(2, 'tools/call', { name: 'echo', arguments: { message: 'hi', user: 'bob' } })) PostHogMcpSpecParkedTool.release << true + expect(alice.join(2)).to eq(alice) expect(alice.value[:result][:content][0][:text]).to eq('done') events = drain_events(client) @@ -415,11 +458,18 @@ def initialize_request(id = 1, version = '2025-06-18') custom = events_named(events, 'inside_tool').first expect(custom[:distinct_id]).to eq('alice') expect(custom[:properties]['$set']).to eq('email' => 'alice@example.com') + ensure + PostHogMcpSpecParkedTool.release&.close + if alice + alice.kill unless alice.join(2) + alice.join + end + PostHogMcpSpecParkedTool.analytics = nil end it 'shrinks a payload a before_send hook grew past the transport limit' do before_send = lambda do |payload| - payload['properties']['bloat'] = 'x' * 40_000 + payload['properties']['bloat'] = '😀"\\' * 10_000 payload end described_class.instrument(server, client, before_send: before_send, enable_conversation_id: false) @@ -430,17 +480,28 @@ def initialize_request(id = 1, version = '2025-06-18') expect(events.map { |e| e[:event] }).to eq(['$mcp_initialize', '$mcp_tool_call']) events.each do |event| expect(JSON.generate(event).bytesize).to be <= PostHog::Defaults::Message::MAX_BYTES - expect(event[:properties]['bloat']).to start_with('x') + expect(event[:properties]['bloat']).to start_with('😀') end + batch = PostHog::MessageBatch.new(events.length) + events.each { |event| batch << event } + expect(batch.length).to eq(events.length) end - it 'never lets analytics failures reach the tool' do - described_class.instrument(server, client, identify: ->(_r, _e) { raise 'identify exploded' }, - event_properties: ->(_r, _e) { raise 'props exploded' }, - intent_fallback: ->(_r, _e) { raise 'intent exploded' }) - result = server.handle(rpc(1, 'tools/call', { name: 'echo', arguments: { message: 'hi' } })) - expect(result[:result][:content][0][:text]).to eq('Echo: hi') - expect(drain_events(client).map { |e| e[:event] }).to include('$mcp_tool_call') + %i[identify event_properties intent_fallback].each do |callback| + it "never lets #{callback} failures reach the tool" do + calls = 0 + failing_callback = lambda do |_request, _extra| + calls += 1 + raise "#{callback} exploded" + end + described_class.instrument(server, client, **{ callback => failing_callback }) + + result = server.handle(rpc(1, 'tools/call', { name: 'echo', arguments: { message: 'hi' } })) + + expect(result[:result][:content][0][:text]).to eq('Echo: hi') + expect(calls).to be >= 1 + expect(drain_events(client).map { |event| event[:event] }).to include('$mcp_tool_call') + end end it 'uses intent_fallback when no context arrives' do diff --git a/spec/posthog/mcp/sanitization_spec.rb b/spec/posthog/mcp/sanitization_spec.rb index ec255ada..fe0c8028 100644 --- a/spec/posthog/mcp/sanitization_spec.rb +++ b/spec/posthog/mcp/sanitization_spec.rb @@ -285,14 +285,14 @@ it 'leaves frames without source context, and odd stacktrace shapes, alone' do bare = { 'filename' => 'lib/gem.rb', 'lineno' => 3, 'in_app' => false } - %w[frames].each do |_| - event = { 'error' => { '$exception_list' => [{ 'value' => 'x', 'stacktrace' => { 'frames' => [bare] } }] } } - expect(described_class.sanitize_event(event)['error']['$exception_list'][0]['stacktrace']['frames']) - .to eq([bare]) - end + event = { 'error' => { '$exception_list' => [{ 'value' => 'x', 'stacktrace' => { 'frames' => [bare] } }] } } + expect(described_class.sanitize_event(event)['error']['$exception_list'][0]['stacktrace']['frames']) + .to eq([bare]) [nil, 'nope', { 'frames' => 'nope' }, { 'frames' => [nil, 42] }].each do |stacktrace| event = { 'error' => { '$exception_list' => [{ 'value' => 'x', 'stacktrace' => stacktrace }] } } - expect { described_class.sanitize_event(event) }.not_to raise_error + original = Marshal.load(Marshal.dump(event)) + expect(described_class.sanitize_event(event)['error']['$exception_list'][0]['stacktrace']).to eq(stacktrace) + expect(event).to eq(original) end expect(described_class.sanitize_event('error' => { '$exception_list' => ['not a hash'] })['error']) .to eq('$exception_list' => ['not a hash']) diff --git a/spec/posthog/mcp/session_token_spec.rb b/spec/posthog/mcp/session_token_spec.rb index c8182e0a..9f64479d 100644 --- a/spec/posthog/mcp/session_token_spec.rb +++ b/spec/posthog/mcp/session_token_spec.rb @@ -19,13 +19,18 @@ def wire(value) it 'round-trips, omits absent fields, and clamps client fields to 200 chars' do token = described_class.encode(session_id: 'ses_0199aabb') + expect(JSON.parse(Base64.urlsafe_decode64(token))).to eq('sid' => 'ses_0199aabb') expect(described_class.decode(token).to_h).to eq(session_id: 'ses_0199aabb', client_name: nil, client_version: nil, protocol_version: nil) - token = described_class.encode(session_id: 'ses_x', client_name: 'a' * 500, client_version: 'b' * 500) - decoded = described_class.decode(token) - expect(decoded.client_name.length).to eq(200) - expect(decoded.client_version.length).to eq(200) + token = described_class.encode(session_id: 'ses_x', client_name: 'a' * 500, client_version: 'b' * 500, + protocol_version: 'c' * 500) + expect(JSON.parse(Base64.urlsafe_decode64(token))).to eq( + 'sid' => 'ses_x', 'cn' => 'a' * 200, 'cv' => 'b' * 200, 'pv' => 'c' * 200 + ) + decoded = described_class.decode(wire({ sid: 'ses_x', cn: 'a' * 500, cv: 'b' * 500, pv: 'c' * 500 })) + expect(decoded.to_h).to eq(session_id: 'ses_x', client_name: 'a' * 200, client_version: 'b' * 200, + protocol_version: 'c' * 200) token = described_class.encode(session_id: 'ses_0199aabb', client_name: 'Клиент 😀 客户端', client_version: '1.0') expect(token).to match(/\A[A-Za-z0-9_-]+\z/) @@ -50,6 +55,11 @@ def wire(value) expect(described_class.decode(huge)).to be_nil end + it 'accepts a 128-character session id but rejects 129 characters' do + expect(described_class.decode(wire({ sid: 's' * 128 })).session_id).to eq('s' * 128) + expect(described_class.decode(wire({ sid: 's' * 129 }))).to be_nil + end + it 'drops malformed client fields but keeps the session id' do decoded = described_class.decode(wire({ sid: 'ses_x', cn: 42, cv: {} })) expect(decoded.session_id).to eq('ses_x') diff --git a/spec/posthog/mcp/truncation_spec.rb b/spec/posthog/mcp/truncation_spec.rb index d2f755d8..ea2b81f0 100644 --- a/spec/posthog/mcp/truncation_spec.rb +++ b/spec/posthog/mcp/truncation_spec.rb @@ -76,7 +76,7 @@ it 'bounds user-supplied properties so the event fits the core client budget' do event = described_class.truncate_event('event_type' => 'custom', 'properties' => { 'rows' => (1..10_000).to_a }) - expect(described_class.json_byte_size(event)).to be <= described_class::MAX_EVENT_BYTES + expect(JSON.generate(event).bytesize).to be <= described_class::MAX_EVENT_BYTES expect(event['properties']['rows'].length).to be <= described_class::MAX_BREADTH + 1 end @@ -88,7 +88,7 @@ event = described_class.truncate_event('response' => content) expect(event['response']['content'][0]['text']).to end_with('...') - expect(described_class.json_byte_size(event)).to be <= described_class::MAX_EVENT_BYTES + expect(JSON.generate(event).bytesize).to be <= described_class::MAX_EVENT_BYTES end it 'keeps events within the byte budget without mutating the input' do @@ -105,9 +105,9 @@ cases.each do |input| event = input.merge('timestamp' => timestamp, 'event_type' => 'mcp:tools/call', 'is_error' => false) before = Marshal.load(Marshal.dump(event)) - expect(described_class.json_byte_size(event)).to be > described_class::MAX_EVENT_BYTES + expect(JSON.generate(event).bytesize).to be > described_class::MAX_EVENT_BYTES out = described_class.truncate_event(event) - expect(described_class.json_byte_size(out)).to be <= described_class::MAX_EVENT_BYTES + expect(JSON.generate(out).bytesize).to be <= described_class::MAX_EVENT_BYTES expect(out['timestamp']).to eq(timestamp) expect(out['event_type']).to eq('mcp:tools/call') expect(event).to eq(before) diff --git a/spec/posthog/message_batch_spec.rb b/spec/posthog/message_batch_spec.rb index fd63175c..73bde3e8 100644 --- a/spec/posthog/message_batch_spec.rb +++ b/spec/posthog/message_batch_spec.rb @@ -10,6 +10,7 @@ module PostHog it 'appends messages' do subject << { 'a' => 'b' } expect(subject.length).to eq(1) + expect(JSON.parse(subject.to_json)).to eq([{ 'a' => 'b' }]) end it 'rejects messages that exceed the maximum allowed size' do @@ -21,6 +22,24 @@ module PostHog end end + describe '#clear' do + it 'resets both the messages and the accumulated byte size' do + stub_const('PostHog::Defaults::Message::MAX_BYTES', 20) + stub_const('PostHog::Defaults::MessageBatch::MAX_BYTES', 40) + 3.times { subject << { a: 'b' } } + expect(subject).to be_full + + subject.clear + + expect(subject).to be_empty + expect(subject.length).to eq(0) + expect(JSON.parse(subject.to_json)).to eq([]) + subject << { c: 'd' } + expect(subject).not_to be_full + expect(JSON.parse(subject.to_json)).to eq([{ 'c' => 'd' }]) + end + end + describe '#full?' do it 'returns true once item count is exceeded' do 99.times { subject << { a: 'b' } } diff --git a/spec/posthog/rails/exception_mechanism_spec.rb b/spec/posthog/rails/exception_mechanism_spec.rb index 24dfc3ec..c00ebb9c 100644 --- a/spec/posthog/rails/exception_mechanism_spec.rb +++ b/spec/posthog/rails/exception_mechanism_spec.rb @@ -256,6 +256,7 @@ def perform_now it 'prevents Rails error subscriber from capturing the same job exception again' do PostHog::Rails.config.auto_instrument_active_job = true + allow(PostHog).to receive(:capture_exception).and_return(true) error = nil begin @@ -264,6 +265,8 @@ def perform_now error = e end + expect(error).to be_a(StandardError) + expect(error.message).to eq('job failed') PostHog::Rails::ErrorSubscriber.new.report( error, handled: false, diff --git a/spec/posthog/rails/logs/appender_spec.rb b/spec/posthog/rails/logs/appender_spec.rb index be6a0498..a7554c0f 100644 --- a/spec/posthog/rails/logs/appender_spec.rb +++ b/spec/posthog/rails/logs/appender_spec.rb @@ -159,6 +159,11 @@ def on_emit(**kwargs) end describe 'rate limiting' do + before do + allow(Process).to receive(:clock_gettime).and_call_original + allow(Process).to receive(:clock_gettime).with(Process::CLOCK_MONOTONIC).and_return(0) + end + let(:rate_limiter) { PostHog::Rails::Logs::RateLimiter.new(2) } subject(:appender) { described_class.new(otel_logger, level: Logger::INFO, rate_limiter: rate_limiter) } @@ -173,6 +178,7 @@ def on_emit(**kwargs) 5.times { |i| appender.info("msg #{i}") } expect(otel_logger.emitted.size).to eq(3) + expect(otel_logger.emitted.first(2).map { |record| record[:body] }).to eq(['msg 0', 'msg 1']) notice = otel_logger.emitted.last expect(notice[:body]).to include('rate cap reached (2 records/minute)') expect(notice[:severity_text]).to eq('WARN') @@ -189,6 +195,11 @@ def on_emit(**kwargs) end describe 'before_send' do + before do + allow(Process).to receive(:clock_gettime).and_call_original + allow(Process).to receive(:clock_gettime).with(Process::CLOCK_MONOTONIC).and_return(0) + end + it 'sends the record returned by the callback' do before_send = proc { |record| record.merge(body: record[:body].gsub('secret', '[redacted]')) } appender = described_class.new(otel_logger, level: Logger::INFO, before_send: before_send) @@ -282,11 +293,17 @@ def on_emit(**kwargs) end it 'drops the record (rather than sending it unscrubbed) when the callback raises' do - before_send = proc { |_record| raise 'scrubber bug' } + before_send = proc do |record| + raise 'scrubber bug' if record[:body].include?('secret') + + record + end appender = described_class.new(otel_logger, level: Logger::INFO, before_send: before_send) expect { appender.info('the secret token') }.not_to raise_error expect(otel_logger.emitted).to be_empty + appender.info('safe later record') + expect(otel_logger.emitted.map { |record| record[:body] }).to eq(['safe later record']) end # Cross-SDK spec: before_send runs before the rate cap, so callback-dropped @@ -338,7 +355,8 @@ def on_emit(**kwargs) broadcast = ActiveSupport::BroadcastLogger.new(Logger.new(IO::NULL), appender) expect { broadcast.info('outer') }.not_to raise_error - expect(otel_logger.emitted.map { |r| r[:body] }).to eq(['outer']) + broadcast.info('later') + expect(otel_logger.emitted.map { |r| r[:body] }).to eq(%w[outer later]) end end diff --git a/spec/posthog/rails/logs/fork_safety_spec.rb b/spec/posthog/rails/logs/fork_safety_spec.rb index 20945e1c..5fbd8bdd 100644 --- a/spec/posthog/rails/logs/fork_safety_spec.rb +++ b/spec/posthog/rails/logs/fork_safety_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require 'io/wait' $LOAD_PATH.unshift File.expand_path('../../../../posthog-rails/lib', __dir__) @@ -21,7 +22,12 @@ # pid-change detection to restart itself in forked workers; this spec pins # that behavior so an OTel SDK regression (or a swap to a processor without # fork detection) is caught in CI rather than as silently unflushed logs. -RSpec.describe 'PostHog Logs fork safety', if: otel_available && Process.respond_to?(:fork) do +RSpec.describe 'PostHog Logs fork safety' do + before do + skip 'Install the gemfiles/otel.gemfile bundle to run OTel fork integration' unless otel_available + skip 'Fork is unavailable on this platform' unless Process.respond_to?(:fork) + end + # Exporter that writes each record body to a pipe, so exports happening # inside the forked child are observable from the parent. let(:exporter_class) do @@ -48,9 +54,10 @@ def shutdown(timeout: nil) # rubocop:disable Lint/UnusedMethodArgument it 'exports records logged in a forked worker (BatchLogRecordProcessor restarts post-fork)' do reader, writer = IO.pipe + acknowledgement_reader, acknowledgement_writer = IO.pipe provider = OpenTelemetry::SDK::Logs::LoggerProvider.new provider.add_log_record_processor( - OpenTelemetry::SDK::Logs::Export::BatchLogRecordProcessor.new(exporter_class.new(writer)) + OpenTelemetry::SDK::Logs::Export::BatchLogRecordProcessor.new(exporter_class.new(writer), schedule_delay: 10) ) appender = PostHog::Rails::Logs::Appender.new( provider.logger(name: 'posthog-rails-test'), @@ -60,20 +67,38 @@ def shutdown(timeout: nil) # rubocop:disable Lint/UnusedMethodArgument # Emit pre-fork so the processor's worker thread starts in the parent — # the preloaded-server scenario where that thread is dead in the child. appender.info('from parent') - provider.force_flush + expect(provider.force_flush).to eq(OpenTelemetry::SDK::Logs::Export::SUCCESS) + expect(reader.wait_readable(5)).not_to be_nil + expect(reader.gets).to eq("from parent\n") pid = fork do reader.close + acknowledgement_writer.close appender.info('from child') - provider.force_flush + acknowledgement_reader.read(1) exit!(0) # skip at_exit/RSpec hooks inherited from the parent end - writer.close - _, status = Process.wait2(pid) + acknowledgement_reader.close + expect(reader.wait_readable(5)).not_to be_nil + expect(reader.gets).to eq("from child\n") + acknowledgement_writer.write('x') + status = nil + eventually do + result = Process.wait2(pid, Process::WNOHANG) + expect(result).not_to be_nil + status = result.last + end + pid = nil expect(status).to be_success - expect(reader.read).to include('from child') ensure - reader&.close + if pid + Process.kill('KILL', pid) + Process.wait(pid) + end + provider&.shutdown(timeout: 1) + [reader, writer, acknowledgement_reader, acknowledgement_writer].compact.each do |io| + io.close unless io.closed? + end end end diff --git a/spec/posthog/rails/logs/rate_limiter_spec.rb b/spec/posthog/rails/logs/rate_limiter_spec.rb index d2abbe2a..cb2992a5 100644 --- a/spec/posthog/rails/logs/rate_limiter_spec.rb +++ b/spec/posthog/rails/logs/rate_limiter_spec.rb @@ -33,8 +33,13 @@ def stub_monotonic_time(seconds) 4.times { limiter.record } expect(limiter.record).to eq(:reject) - stub_monotonic_time(described_class::WINDOW_SECONDS) - expect(limiter.record).to eq(:allow) + stub_monotonic_time(59.999) + expect(limiter.record).to eq(:reject) + + stub_monotonic_time(60.0) + expect(Array.new(3) { limiter.record }).to all(eq(:allow)) + expect(limiter.record).to eq(:reject_first) + expect(limiter.record).to eq(:reject) end it 'counts concurrent records without losing increments' do diff --git a/spec/posthog/rails/logs/real_export_spec.rb b/spec/posthog/rails/logs/real_export_spec.rb index 325cf6ac..e1cc1d90 100644 --- a/spec/posthog/rails/logs/real_export_spec.rb +++ b/spec/posthog/rails/logs/real_export_spec.rb @@ -23,11 +23,17 @@ # >= 0.6.0, so an older pairing raises NoMethodError during encode — no HTTP # request is ever made and the expectations below fail. (logs-sdk >= 0.6.0 # requires Ruby 3.3+, so this spec only runs where those gems are installed.) -RSpec.describe 'PostHog Logs real OTLP export', if: otel_available do +RSpec.describe 'PostHog Logs real OTLP export' do + before { skip 'Install the gemfiles/otel.gemfile bundle to run real OTLP integration' unless otel_available } + let(:endpoint) { 'https://logs.example.test/i/v1/logs' } it 'encodes a record and POSTs it to the OTLP endpoint with the bearer token' do - stub = stub_request(:post, endpoint).to_return(status: 200, body: '') + request_body = nil + stub = stub_request(:post, endpoint).to_return do |request| + request_body = request.headers['Content-Encoding'] == 'gzip' ? Zlib.gunzip(request.body) : request.body + { status: 200, body: '' } + end provider = OpenTelemetry::SDK::Logs::LoggerProvider.new exporter = OpenTelemetry::Exporter::OTLP::Logs::LogsExporter.new( @@ -46,12 +52,20 @@ # force_flush exports synchronously; SUCCESS proves encode + transport ran. expect(provider.force_flush).to eq(OpenTelemetry::SDK::Logs::Export::SUCCESS) - expect(stub).to have_been_requested + expect(stub).to have_been_requested.once + payload = Opentelemetry::Proto::Collector::Logs::V1::ExportLogsServiceRequest.decode(request_body) + scopes = payload.resource_logs.flat_map { |resource| resource.scope_logs.to_a } + expect(scopes.map { |scope| scope.scope.name }).to eq(['posthog-rails-test']) + records = scopes.flat_map { |scope| scope.log_records.to_a } + expect(records.map { |record| record.body.string_value }).to eq(['real export smoke']) + expect(records.map(&:severity_number)).to eq([:SEVERITY_NUMBER_INFO]) expect(a_request(:post, endpoint).with( headers: { 'Authorization' => 'Bearer phc_test', 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once + ensure + provider&.shutdown(timeout: 1) end end diff --git a/spec/posthog/rails/logs/setup_spec.rb b/spec/posthog/rails/logs/setup_spec.rb index f1e7d8e3..d56fdcd9 100644 --- a/spec/posthog/rails/logs/setup_spec.rb +++ b/spec/posthog/rails/logs/setup_spec.rb @@ -35,7 +35,7 @@ expect(described_class.install).to be_nil described_class.install # idempotent; should not warn again - expect(logger).to have_received(:warn).once + expect(logger).to have_received(:warn).with(/OpenTelemetry gems are missing/).once end end @@ -51,7 +51,7 @@ PostHog::Logging.logger = logger expect(described_class.install).to be_nil - expect(logger).to have_received(:warn).once + expect(logger).to have_received(:warn).with(/no project token could be resolved/).once end end @@ -154,10 +154,10 @@ end describe '.shutdown' do - it 'bounds the final flush with a timeout so a hung exporter cannot eat the SIGTERM grace period' do + it 'passes a two-second shutdown budget to the provider' do provider = double('provider') described_class.instance_variable_set(:@provider, provider) - expect(provider).to receive(:shutdown).with(timeout: described_class::SHUTDOWN_TIMEOUT_SECONDS) + expect(provider).to receive(:shutdown).with(timeout: 2) described_class.shutdown end diff --git a/spec/posthog/rails/railtie_spec.rb b/spec/posthog/rails/railtie_spec.rb index 0e602f73..c216db9d 100644 --- a/spec/posthog/rails/railtie_spec.rb +++ b/spec/posthog/rails/railtie_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require 'open3' # Load the full Rails stack so the boot-order test can simulate real load order. require 'logger' @@ -25,10 +26,18 @@ after { PostHog.client = nil } it 'allows PostHog.init before Railtie initializers run' do - client = PostHog.init(api_key: 'phc_test', test_mode: true) - - expect(client).to be_a(PostHog::Client) - expect(PostHog.client).to eq(client) + script = <<~RUBY + require 'posthog' + require 'rails' + require 'posthog/rails' + Rails.logger = Logger.new(File::NULL) + client = PostHog.init(api_key: 'phc_test', test_mode: true) + abort 'facade did not install its client' unless client.is_a?(PostHog::Client) && PostHog.client.equal?(client) + client.shutdown + RUBY + stdout, stderr, status = Open3.capture3(RbConfig.ruby, '-Ilib', '-Iposthog-rails/lib', '-e', script) + + expect(status).to be_success, "#{stdout}\n#{stderr}" end end @@ -106,6 +115,18 @@ expect do railtie.instance_exec(app, &initializer.block) end.not_to raise_error + + stack = ActionDispatch::MiddlewareStack.new + stack.use(ActionDispatch::ShowExceptions) + stack.use(ActionDispatch::DebugExceptions) + middleware_proxy.merge_into(stack) + expect(stack.middlewares.map(&:klass)).to eq([ + PostHog::Rails::RequestContext, + ActionDispatch::ShowExceptions, + PostHog::Rails::CaptureExceptions, + ActionDispatch::DebugExceptions, + PostHog::Rails::RescuedExceptionInterceptor + ]) end it 'inserts middleware before and after the target when it is present' do diff --git a/spec/posthog/rails/request_context_spec.rb b/spec/posthog/rails/request_context_spec.rb index cc50b0e8..33e85a18 100644 --- a/spec/posthog/rails/request_context_spec.rb +++ b/spec/posthog/rails/request_context_spec.rb @@ -182,8 +182,8 @@ def call_with(headers = nil, path: '/api/test', **header_keywords, &block) it 'prefers authenticated Rails user context over tracing headers for exceptions' do PostHog::Rails.config.auto_capture_exceptions = true - allow(PostHog).to receive(:capture_exception) do |exception, distinct_id, properties| - client.capture_exception(exception, distinct_id, properties) + allow(PostHog).to receive(:capture_exception) do |*args, **kwargs| + client.capture_exception(*args, **kwargs) end user = Struct.new(:id).new('rails-user') @@ -223,7 +223,9 @@ def current_user ) end.to raise_error(StandardError, 'boom') + expect(client.queued_messages).to eq(1) message = client.dequeue_last_message + expect(message[:properties]['$exception_list'].first['mechanism']['handled']).to be(false) expect(message[:event]).to eq('$exception') expect(message[:distinct_id]).to eq('rails-user') expect(message[:properties]['$session_id']).to eq('exception-session') @@ -257,8 +259,8 @@ def action_name end end - allow(PostHog).to receive(:capture_exception) do |exception, distinct_id, properties| - client.capture_exception(exception, distinct_id, properties) + allow(PostHog).to receive(:capture_exception) do |*args, **kwargs| + client.capture_exception(*args, **kwargs) end [ @@ -305,7 +307,9 @@ def action_name ) end.to raise_error(StandardError, "boom #{scenario.fetch(:description)}") + expect(client.queued_messages).to eq(1) message = client.dequeue_last_message + expect(message[:properties]['$exception_list'].first['mechanism']['handled']).to be(false) expect(message[:event]).to eq('$exception') expect(message[:distinct_id]).to eq(scenario.fetch(:expected_distinct_id)) expect(message[:properties]['$session_id']).to eq('exception-session') @@ -315,8 +319,8 @@ def action_name it 'captures exceptions with tracing context and re-raises' do PostHog::Rails.config.auto_capture_exceptions = true - allow(PostHog).to receive(:capture_exception) do |exception, distinct_id, properties| - client.capture_exception(exception, distinct_id, properties) + allow(PostHog).to receive(:capture_exception) do |*args, **kwargs| + client.capture_exception(*args, **kwargs) end app = lambda do |_env| @@ -335,7 +339,9 @@ def action_name ) end.to raise_error(StandardError, 'boom') + expect(client.queued_messages).to eq(1) message = client.dequeue_last_message + expect(message[:properties]['$exception_list'].first['mechanism']['handled']).to be(false) expect(message[:event]).to eq('$exception') expect(message[:distinct_id]).to eq('exception-user') expect(message[:properties]['$session_id']).to eq('exception-session') @@ -348,8 +354,8 @@ def action_name PostHog::Rails.config.auto_capture_exceptions = true PostHog::Rails.config.use_tracing_headers = false - allow(PostHog).to receive(:capture_exception) do |exception, distinct_id, properties| - client.capture_exception(exception, distinct_id, properties) + allow(PostHog).to receive(:capture_exception) do |*args, **kwargs| + client.capture_exception(*args, **kwargs) end app = lambda do |_env| @@ -368,7 +374,9 @@ def action_name ) end.to raise_error(StandardError, 'boom') + expect(client.queued_messages).to eq(1) message = client.dequeue_last_message + expect(message[:properties]['$exception_list'].first['mechanism']['handled']).to be(false) expect(message[:event]).to eq('$exception') expect(message[:distinct_id]).not_to eq('disabled-header-user') expect(message[:properties]['$process_person_profile']).to be false diff --git a/spec/posthog/response_spec.rb b/spec/posthog/response_spec.rb index d18cf5ab..facbb756 100644 --- a/spec/posthog/response_spec.rb +++ b/spec/posthog/response_spec.rb @@ -18,12 +18,12 @@ module PostHog subject { described_class.new(status, error) } - it 'sets the instance variable status' do - expect(subject.instance_variable_get(:@status)).to eq(status) + it 'exposes the supplied status' do + expect(subject.status).to eq(status) end - it 'sets the instance variable error' do - expect(subject.instance_variable_get(:@error)).to eq(error) + it 'exposes the supplied error' do + expect(subject.error).to eq(error) end end end diff --git a/spec/posthog/send_worker_spec.rb b/spec/posthog/send_worker_spec.rb index b532f9fa..f44bf0f1 100644 --- a/spec/posthog/send_worker_spec.rb +++ b/spec/posthog/send_worker_spec.rb @@ -7,11 +7,26 @@ module PostHog around do |example| PostHog::Transport.stub = true example.call + ensure PostHog::Transport.stub = false end + after do + Array(@worker_threads).each do |worker, thread| + worker.shutdown + thread.kill unless thread.join(1) + thread.join + end + end + + def start_worker(worker) + thread = Thread.new { worker.run } + (@worker_threads ||= []) << [worker, thread] + thread + end + def run_worker_until_idle(worker, queue) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(queue).to be_empty } worker.shutdown expect(worker_thread.join(1)).to eq(worker_thread) @@ -73,15 +88,6 @@ def run_worker_until_idle(worker, queue) end describe '#run' do - before :all do - PostHog::Defaults::Request::BACKOFF = 0.1 - end - - after :all do - PostHog::Defaults::Request.send(:remove_const, :BACKOFF) - PostHog::Defaults::Request::BACKOFF = 30.0 - end - it 'does not error if the request fails' do expect do allow_any_instance_of(PostHog::Transport).to( @@ -102,29 +108,33 @@ def run_worker_until_idle(worker, queue) receive(:send).and_return(PostHog::Response.new(400, 'Some error')) ) + entered = Queue.new + release = Queue.new status = error = nil - on_error = - proc do |yielded_status, yielded_error| - sleep 0.2 # Make this take longer than thread spin-up (below) - status = yielded_status - error = yielded_error - end + on_error = proc do |yielded_status, yielded_error| + entered << true + release.pop + status = yielded_status + error = yielded_error + end queue = Queue.new queue << {} worker = described_class.new(queue, 'secret', on_error: on_error, flush_interval_seconds: 0) - # This is to ensure that Client#flush doesn't finish before calling - # the error handler. - worker_thread = Thread.new { worker.run } - sleep 0.1 # First give thread time to spin-up. - sleep 0.01 while worker.is_requesting? + worker_thread = start_worker(worker) + eventually { expect(entered).not_to be_empty } + expect(worker.is_requesting?).to eq(true) + release << true + eventually { expect(worker.is_requesting?).to eq(false) } worker.shutdown - worker_thread.join(1) + expect(worker_thread.join(1)).to eq(worker_thread) expect(queue).to be_empty expect(status).to eq(400) expect(error).to eq('Some error') + ensure + release&.push(true) end it 'clears the in-flight batch if the error handler raises' do @@ -143,7 +153,7 @@ def run_worker_until_idle(worker, queue) ) worker.instance_variable_set(:@transport, transport) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually do expect(queue).to be_empty expect(worker.is_requesting?).to eq(false) @@ -204,7 +214,9 @@ def bad_message.to_json(*_args) it 'waits for flush_interval_seconds before sending a partial batch' do sends = [] + sent_at = nil allow_any_instance_of(PostHog::Transport).to receive(:send) do |_transport, _api_key, batch| + sent_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) sends << batch.length PostHog::Response.new(200, 'Success') end @@ -214,14 +226,12 @@ def bad_message.to_json(*_args) worker = described_class.new(queue, 'testsecret', batch_size: 10, flush_interval_seconds: 0.05) started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(sends).to eq([1]) } worker.shutdown expect(worker_thread.join(1)).to eq(worker_thread) - elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at - expect(sends).to eq([1]) - expect(elapsed).to be >= 0.05 + expect(sent_at - started_at).to be >= 0.05 end it 'sends immediately when the batch size is reached' do @@ -237,7 +247,7 @@ def bad_message.to_json(*_args) worker = described_class.new(queue, 'testsecret', batch_size: 2, flush_interval_seconds: 60) started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(sends).to eq([2]) } worker.shutdown expect(worker_thread.join(1)).to eq(worker_thread) @@ -259,7 +269,7 @@ def bad_message.to_json(*_args) worker = described_class.new(queue, 'testsecret', batch_size: 2, flush_interval_seconds: 60) started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(worker.is_requesting?).to eq(true) } queue << Requested::CAPTURE.merge(event: 'Second event') @@ -283,7 +293,7 @@ def bad_message.to_json(*_args) queue = Queue.new worker = described_class.new(queue, 'testsecret', batch_size: 1, flush_interval_seconds: 60) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(worker_thread).to be_alive } queue << Requested::CAPTURE @@ -296,28 +306,33 @@ def bad_message.to_json(*_args) it 'does not keep a stale flush request while idle' do sends = [] + sent_at = nil allow_any_instance_of(PostHog::Transport).to receive(:send) do |_transport, _api_key, batch| + sent_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) sends << batch.length PostHog::Response.new(200, 'Success') end queue = Queue.new worker = described_class.new(queue, 'testsecret', batch_size: 10, flush_interval_seconds: 0.05) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(worker_thread).to be_alive } + cleared = Queue.new + allow(worker).to receive(:clear_flush_request_without_lock).and_wrap_original do |method| + method.call + cleared << true + end worker.request_flush - sleep 0.01 - queue << Requested::CAPTURE + eventually { expect(cleared).not_to be_empty } started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + queue << Requested::CAPTURE worker.notify eventually { expect(sends).to eq([1]) } worker.shutdown expect(worker_thread.join(1)).to eq(worker_thread) - elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at - - expect(elapsed).to be >= 0.05 + expect(sent_at - started_at).to be >= 0.05 end it 'flushes immediately when requested' do @@ -331,7 +346,7 @@ def bad_message.to_json(*_args) queue << Requested::CAPTURE worker = described_class.new(queue, 'testsecret', batch_size: 10, flush_interval_seconds: 60) - worker_thread = Thread.new { worker.run } + worker_thread = start_worker(worker) eventually { expect(worker.is_requesting?).to eq(true) } worker.request_flush @@ -351,8 +366,11 @@ def bad_message.to_json(*_args) end it 'returns true if there is a current batch' do + entered = Queue.new + release = Queue.new allow_any_instance_of(PostHog::Transport).to receive(:send) do - sleep(0.2) + entered << true + release.pop PostHog::Response.new(200, 'Success') end @@ -360,13 +378,17 @@ def bad_message.to_json(*_args) queue << Requested::CAPTURE worker = described_class.new(queue, 'testsecret', flush_interval_seconds: 0) - worker_thread = Thread.new { worker.run } - eventually { expect(worker.is_requesting?).to eq(true) } + worker_thread = start_worker(worker) + eventually { expect(entered).not_to be_empty } + expect(worker.is_requesting?).to eq(true) + release << true eventually { expect(worker.is_requesting?).to eq(false) } worker.shutdown - worker_thread.join + expect(worker_thread.join(1)).to eq(worker_thread) expect(worker.is_requesting?).to eq(false) + ensure + release&.push(true) end end end diff --git a/spec/posthog/transport_spec.rb b/spec/posthog/transport_spec.rb index 0a84308c..0fa932ba 100644 --- a/spec/posthog/transport_spec.rb +++ b/spec/posthog/transport_spec.rb @@ -16,18 +16,18 @@ module PostHog before { allow(Net::HTTP).to receive(:new) { net_http } } - it 'sets an initalized Net::HTTP read_timeout' do - expect(net_http).to receive(:use_ssl=) + it 'enables TLS by default' do + expect(net_http).to receive(:use_ssl=).with(true) described_class.new end it 'sets an initalized Net::HTTP read_timeout' do - expect(net_http).to receive(:read_timeout=) + expect(net_http).to receive(:read_timeout=).with(8) described_class.new end it 'sets an initalized Net::HTTP open_timeout' do - expect(net_http).to receive(:open_timeout=) + expect(net_http).to receive(:open_timeout=).with(4) described_class.new end @@ -184,7 +184,7 @@ module PostHog context 'a real request' do RSpec.shared_examples('retried request') do |status_code, body| let(:status_code) { status_code } - let(:body) { body } + let(:response_body) { body } let(:retries) { 4 } let(:backoff_policy) { FakeBackoffPolicy.new([1000, 1000, 1000]) } subject do @@ -200,20 +200,21 @@ module PostHog .times .with(1) .and_return(nil) - subject.send(api_key, batch) + expect(subject.send(api_key, batch).status).to eq(status_code) + expect(subject.instance_variable_get(:@http)).to have_received(:request).exactly(retries).times end end RSpec.shared_examples('non-retried request') do |status_code, body| let(:status_code) { status_code } - let(:body) { body } + let(:response_body) { body } let(:retries) { 4 } - let(:backoff) { 1 } - subject { described_class.new(retries: retries, backoff: backoff) } + subject { described_class.new(retries: retries) } it 'does not retry the request' do expect(subject).to receive(:sleep).never - subject.send(api_key, batch) + expect(subject.send(api_key, batch).status).to eq(status_code) + expect(subject.instance_variable_get(:@http)).to have_received(:request).once end end @@ -318,7 +319,7 @@ module PostHog it_behaves_like('retried request', 500, '{}') it_behaves_like('retried request', 503, '{}') - # All 4xx errors other than 429 (rate limited) must be retried + it_behaves_like('retried request', 408, '{}') it_behaves_like('retried request', 429, '{}') it_behaves_like('non-retried request', 404, '{}') it_behaves_like('non-retried request', 400, '{}') @@ -343,6 +344,24 @@ module PostHog subject.send(api_key, batch) end + [ + ['seconds', '2.5', 2.5], + ['a future HTTP date', 'Wed, 01 Jan 2025 00:00:05 GMT', 5.0], + ['a past HTTP date', 'Tue, 31 Dec 2024 23:59:59 GMT', 1.0], + ['a negative value', '-1', 1.0], + ['an invalid value', 'invalid', 1.0], + ['an empty value', '', 1.0] + ].each do |description, header, delay| + it "handles Retry-After with #{description}" do + allow(Time).to receive(:now).and_return(Time.utc(2025, 1, 1)) + allow(response).to receive(:[]).with('Retry-After').and_return(header) + expect(subject).to receive(:sleep).once.with(delay) + + expect(subject.send(api_key, batch).status).to eq(429) + expect(subject.instance_variable_get(:@http)).to have_received(:request).twice + end + end + it 'does not reuse a stale Retry-After header after retries are exhausted' do http = subject.instance_variable_get(:@http) rate_limited_response = Net::HTTPResponse.new(http_version, 429, 'Too Many Requests') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c227ba00..6563179a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,6 +8,13 @@ config.before(:each) do PostHog::Logging.logger = Logger.new(File::NULL) # Suppress all logging PostHog::Client.reset_instance_tracking! + @existing_posthog_clients = ObjectSpace.each_object(PostHog::Client).to_a + end + + config.after(:each) do + (ObjectSpace.each_object(PostHog::Client).to_a - @existing_posthog_clients).each do |client| + client.shutdown(timeout: 0) + end end end @@ -83,12 +90,12 @@ module AsyncHelper def eventually(options = {}) timeout = options[:timeout] || 2 interval = options[:interval] || 0.1 - time_limit = Time.now + timeout + time_limit = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout loop do yield return rescue RSpec::Expectations::ExpectationNotMetError => e - raise e if Time.now >= time_limit + raise e if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= time_limit sleep interval end