Skip to content

feat: add panic autocapture#130

Closed
cat-ph wants to merge 10 commits into
mainfrom
ci/rust-panic-autocapture
Closed

feat: add panic autocapture#130
cat-ph wants to merge 10 commits into
mainfrom
ci/rust-panic-autocapture

Conversation

@cat-ph

@cat-ph cat-ph commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add process-wide install_panic_hook panic autocapture for the error-tracking feature (now rebased onto main after feat: add manual rust error tracking #129 merged)
  • panics are captured personlessly as $exception events: the panic payload as the exception value, panic-site $exception_panic_file/_line/_column properties from PanicInfo::location() (compiler-provided, survives release builds without debug info), and a call-site stacktrace honoring capture_stacktrace
  • capture is best-effort and panic-safe: the event is built on the hook thread, then sent synchronously from a dedicated sender thread (panic count is thread-local, so a panic anywhere in the HTTP stack becomes a joinable thread panic instead of a process abort), bounded by panic_capture_timeout_seconds (default 2s); the previously installed hook always runs afterwards
  • before_send hooks are deliberately bypassed for panic captures: hooks run arbitrary user code that cannot safely execute inside a panic hook (a hook mutex may be held mid-panic by this thread, and a nested panic aborts the process); capture failures go to stderr rather than tracing for the same reason
  • installing the hook twice returns Error::PanicHookAlreadyInstalled

Test plan

  • cargo fmt --check and cargo clippy --all-targets -- -D warnings (default and --features capture-v1)
  • cargo test across the four feature configs: default, capture-v1, --no-default-features --features error-tracking, --no-default-features --features error-tracking,capture-v1
  • regression coverage: personless capture + previous-hook chaining, disabled-client no-send, panics on tokio runtime threads (multi-thread and current_thread), inline-frame expansion and internal-frame stripping

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/error_tracking.rs:583-591
**Duplicate of `v0_capture::prepare_event`**

`prepare_panic_event` is a byte-for-byte copy of the `pub(crate) fn prepare_event` already in `src/client/v0_capture.rs`. If either grows in the future (e.g. a new client-level default is added) the other will silently lag behind. Consider reusing `v0_capture::prepare_event` directly here instead.

### Issue 2 of 2
src/error_tracking.rs:505-517
**Near-identical to `capture_application_stacktrace`**

`capture_panic_stacktrace` and `capture_application_stacktrace` (lines 491–503) are structurally identical — they differ only in the predicate passed to the "skip internal frames" loop. Extracting a single `capture_stacktrace_with_filter(options, is_internal: impl Fn(&str) -> bool)` helper would satisfy OnceAndOnlyOnce and make future changes to the trimming logic (e.g. the `trim_to_max_frames` call) apply to both paths automatically.

Reviews (1): Last reviewed commit: "feat: add panic autocapture" | Re-trigger Greptile

Comment thread src/error_tracking.rs Outdated
Comment thread src/error_tracking.rs Outdated
@cat-ph cat-ph force-pushed the ci/rust-error-tracking-manual branch from 240d4dd to a89e17f Compare June 9, 2026 12:59
@cat-ph cat-ph force-pushed the ci/rust-panic-autocapture branch 2 times, most recently from 9989826 to c902fee Compare June 9, 2026 13:29
@cat-ph cat-ph mentioned this pull request Jun 9, 2026
5 tasks
@cat-ph cat-ph force-pushed the ci/rust-panic-autocapture branch from c902fee to 2d9b320 Compare June 10, 2026 15:06
Base automatically changed from ci/rust-error-tracking-manual to main June 12, 2026 13:05
cat-ph added 7 commits June 12, 2026 16:10
Panic events bypassed the configured before_send hooks that every client
capture path applies, so redaction/drop hooks never saw autocaptured panic
payloads. Route the panic sender through the shared apply_before_send_hooks
(widened to pub(crate)) after V0 prep, mirroring client ordering; a hook
returning None drops the event.

Also pins delivery from Tokio runtime threads with a regression test (both
multi-thread worker and current-thread block_on flavors): review flagged
reqwest::blocking as panicking in async contexts, which does not reproduce
with reqwest 0.13's channel-backed blocking client — the test guards the
assumption.
Running before_send hooks inside the panic hook (added one commit ago) was
unsound, verified empirically: a hook's mutex can still be held mid-panic by
the very thread the hook fires on (hooks run before unwinding releases the
guard), and any panic raised inside a panic hook aborts the process -
catch_unwind cannot intervene there. Panic captures now deliberately bypass
before_send, documented on install_panic_hook.

The HTTP send moves to a dedicated thread for the same reason: the panic
count is thread-local, so a panic anywhere in the sender becomes a joinable
thread panic instead of a process abort. Spawned via thread::Builder so a
spawn failure surfaces as an error rather than a panic on the hook thread.
The hook reported capture failures through tracing::debug!, which
dispatches arbitrary subscriber code on the panicking thread - the same
re-entrancy hazard before_send is excluded for (a subscriber panic
aborts the process, a held writer lock deadlocks). Write the failure
note to stderr instead, like the default hook that runs right after.

The internal-frame filter matched the unwind shim only by its Mach-O
spelling (___rust_try); on ELF the symbol is __rust_try, so Linux
traces kept the shim as the top frame. Match the common suffix.
@cat-ph cat-ph force-pushed the ci/rust-panic-autocapture branch from 2d9b320 to 73b3c9a Compare June 12, 2026 13:27
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

posthog-rs-v0 Compliance Report

Date: 2026-06-12 17:58:06 UTC
Duration: 16921ms

⚠️ Some Tests Failed

31/45 tests passed, 14 failed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 123ms
Format Validation.Event Has Uuid 130ms
Format Validation.Event Has Lib Properties 130ms
Format Validation.Distinct Id Is String 121ms
Format Validation.Token Is Present 129ms
Format Validation.Custom Properties Preserved 121ms
Format Validation.Event Has Timestamp 128ms
Retry Behavior.Retries On 503 5728ms
Retry Behavior.Does Not Retry On 400 2121ms
Retry Behavior.Does Not Retry On 401 2133ms
Retry Behavior.Respects Retry After Header 8099ms
Retry Behavior.Implements Backoff 15703ms
Retry Behavior.Retries On 500 5299ms
Retry Behavior.Retries On 502 5288ms
Retry Behavior.Retries On 504 5286ms
Retry Behavior.Max Retries Respected 16493ms
Deduplication.Generates Unique Uuids 93ms
Deduplication.Preserves Uuid On Retry 5218ms
Deduplication.Preserves Uuid And Timestamp On Retry 10628ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 5224ms
Deduplication.No Duplicate Events In Batch 87ms
Deduplication.Different Events Have Different Uuids 49ms
Compression.Sends Gzip When Enabled 47ms
Batch Format.Uses Proper Batch Structure 30ms
Batch Format.Flush With No Events Sends Nothing 36ms
Batch Format.Multiple Events Batched Together 68ms
Error Handling.Does Not Retry On 403 2040ms
Error Handling.Does Not Retry On 413 2031ms
Error Handling.Retries On 408 5237ms

Feature_Flags Tests

⚠️ 2/16 tests passed, 14 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 13ms
Request Payload.Flags Request Uses V2 Query Param 11ms
Request Payload.Flags Request Hits Flags Path Not Decide 12ms
Request Payload.Flags Request Omits Authorization Header 12ms
Request Payload.Token In Flags Body Matches Init 12ms
Request Payload.Groups Round Trip 11ms
Request Payload.Groups Default To Empty Object 13ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It 11ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 12ms
Request Payload.Disable Geoip Omitted Defaults To False 11ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 12ms
Request Lifecycle.No Flags Request On Init Alone 11ms
Request Lifecycle.No Flags Request On Normal Capture 15ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 12ms
Request Lifecycle.Mock Response Value Is Returned To Caller 12ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 12ms

Failures

request_payload.request_with_person_properties_device_id

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-fdf96f0b-52f0-40c1-96ad-43fe5607d2fc'

request_payload.flags_request_uses_v2_query_param

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-65d08f37-3246-4fe1-98ec-65d05fe28b69'

request_payload.flags_request_hits_flags_path_not_decide

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-db10c698-d44e-4d86-8088-5bb01a95c0c6'

request_payload.flags_request_omits_authorization_header

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-738106bd-e57c-4cb1-81df-4892c89bc323'

request_payload.token_in_flags_body_matches_init

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-ef182de2-810a-48d3-96bd-0fc379302236'

request_payload.groups_round_trip

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-d5b3e6de-6e50-4132-9877-7a0276b2b726'

request_payload.groups_default_to_empty_object

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-6ddbebea-312c-4424-9c6e-b18d566b0f76'

request_payload.person_properties_distinct_id_auto_populated_when_caller_omits_it

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-b1c7e127-6357-46b8-92f8-00fd57eb6b49'

request_payload.disable_geoip_false_propagates_as_geoip_disable_false

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-426ef0ab-24a7-4328-b9ab-b9bae62161ff'

request_payload.disable_geoip_omitted_defaults_to_false

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-7ddc219c-7813-4cd7-9a07-b9df7515166f'

request_payload.flag_keys_to_evaluate_contains_only_requested_key

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-d8270f31-8e7a-4304-b129-7d77ed4cf593'

request_lifecycle.two_flag_calls_produce_two_remote_requests

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-39484d29-7c34-4a58-8044-24c3146c7870'

request_lifecycle.mock_response_value_is_returned_to_caller

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-a650bba4-75c4-48d2-9646-60e676c9e5fb'

side_effect_events.get_feature_flag_captures_feature_flag_called_event

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-7370c81a-9e75-459a-b11d-8e7b4dbe1af9'

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

posthog-rs-v1 Compliance Report

Date: 2026-06-12 17:58:28 UTC
Duration: 26994ms

⚠️ Some Tests Failed

96/110 tests passed, 14 failed


Capture_V1 Tests

94/94 tests passed

View Details
Test Status Duration
Endpoint And Method.Targets V1 Endpoint 130ms
Endpoint And Method.Does Not Use Legacy Endpoints 136ms
Required Headers.Has Authorization Bearer Header 129ms
Required Headers.Has Content Type Json 134ms
Required Headers.Has Posthog Sdk Info Format 129ms
Required Headers.Has Posthog Attempt Header 133ms
Required Headers.Has Posthog Request Id 129ms
Required Headers.Has Posthog Request Timestamp 130ms
Required Headers.Has User Agent 134ms
Body Format.Body Has Created At And Batch 132ms
Body Format.No Api Key In Body 133ms
Body Format.No Sent At In Body 130ms
Event Format.Event Has Required Root Fields 123ms
Event Format.Event Uuid Is Valid 129ms
Event Format.Event Timestamp Is Rfc3339 128ms
Event Format.Distinct Id Is String 124ms
Event Format.Distinct Id At Root Not Properties 118ms
Event Format.Custom Properties Preserved 119ms
Event Format.Set Properties Preserved 126ms
Event Format.Set Once Properties Preserved 125ms
Event Format.Groups Properties Preserved 121ms
Event Format.Sdk Generates Uuid If Not Provided 120ms
Event Format.Event Has Required Root Fields Batch 134ms
Event Format.Event Uuid Is Valid Batch 137ms
Event Format.Event Timestamp Is Rfc3339 Batch 136ms
Event Format.Distinct Id Is String Batch 128ms
Event Format.Distinct Id At Root Not Properties Batch 132ms
Event Format.Custom Properties Preserved Batch 133ms
Event Format.Set Properties Preserved Batch 152ms
Event Format.Set Once Properties Preserved Batch 132ms
Event Format.Groups Properties Preserved Batch 134ms
Event Format.Sdk Generates Uuid If Not Provided Batch 143ms
Batch Behavior.Multiple Events In Single Batch 188ms
Batch Behavior.Batch Envelope Smoke 183ms
Batch Behavior.Flush With No Events Sends Nothing 105ms
Batch Behavior.Flush At Triggers Batch 1123ms
Batch Behavior.Created At Reflects Batch Creation Time 113ms
Deduplication.Generates Unique Uuids 177ms
Deduplication.Different Events Same Content Different Uuids 131ms
Deduplication.Preserves Uuid On Retry 6102ms
Deduplication.Preserves Timestamp On Retry 6080ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 6123ms
Deduplication.No Duplicate Events In Batch 124ms
Header Behavior On Retry.Attempt Header Starts At One 64ms
Header Behavior On Retry.Attempt Header Increments On Retry 12065ms
Header Behavior On Retry.Request Id Preserved On Retry 6059ms
Header Behavior On Retry.Different Requests Have Different Request Ids 2070ms
Header Behavior On Retry.Request Timestamp Changes On Retry 6062ms
Response Format Validation.Success Response Has Uuid Keyed Results 55ms
Response Format Validation.Success Response Has Ok For Each Event 32ms
Response Format Validation.Success No Retry After When All Ok 30ms
Response Format Validation.Success Retry After Present When Retry Events 1031ms
Response Format Validation.Success No Retry After When Drop Only 29ms
Response Format Validation.Response Echoes Request Id 16ms
Retry Behavior.Retries On 408 6019ms
Retry Behavior.Retries On 500 6021ms
Retry Behavior.Retries On 503 7023ms
Retry Behavior.Retries On 504 6023ms
Retry Behavior.Retryable Errors Have Retry After 3022ms
Retry Behavior.Respects Retry After On Retryable Error 11021ms
Retry Behavior.Does Not Retry On 400 2040ms
Retry Behavior.Does Not Retry On 401 2038ms
Retry Behavior.Does Not Retry On 402 2035ms
Retry Behavior.Does Not Retry On 413 2019ms
Retry Behavior.Does Not Retry On 415 2018ms
Retry Behavior.Non Retryable Errors Have No Retry After 2048ms
Retry Behavior.Implements Backoff 17067ms
Retry Behavior.Max Retries Respected 18064ms
Partial Batch Handling.Handles 200 Full Success 2044ms
Partial Batch Handling.Handles 200 With All Ok 3019ms
Partial Batch Handling.Does Not Retry Dropped Events 3019ms
Partial Batch Handling.Does Not Retry Limited Events 3020ms
Partial Batch Handling.Prunes Ok Events On Partial Retry 6023ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry 6046ms
Partial Batch Handling.Retries Only Retry Events From Partial 6041ms
Partial Batch Handling.Partial Retry Preserves Uuids 6024ms
Partial Batch Handling.Partial Retry Attempt Header Increments 6022ms
Partial Batch Handling.Partial Retry Request Id Preserved 6024ms
Partial Batch Handling.Respects Retry After On Partial 8022ms
Partial Batch Handling.Unknown Result Treated As Terminal 3019ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry 3045ms
Compression.Sends Gzip Content Encoding 28ms
Compression.No Content Encoding When Disabled 17ms
Compression.Compressed Body Is Decompressible 16ms
Error Handling.Does Not Retry On Unknown 4Xx 2018ms
Event Options.Cookieless Mode Override 17ms
Event Options.Disable Skew Correction Override 16ms
Event Options.Process Person Profile Override 16ms
Event Options.Product Tour Id Override 15ms
Event Options.Unset Options Omitted 15ms
Event Options.Options Override In Batch 18ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties 16ms
Geoip And Historical Migration.Historical Migration Set In Body 15ms
Geoip And Historical Migration.Historical Migration Absent By Default 15ms

Feature_Flags Tests

⚠️ 2/16 tests passed, 14 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 13ms
Request Payload.Flags Request Uses V2 Query Param 13ms
Request Payload.Flags Request Hits Flags Path Not Decide 15ms
Request Payload.Flags Request Omits Authorization Header 32ms
Request Payload.Token In Flags Body Matches Init 23ms
Request Payload.Groups Round Trip 24ms
Request Payload.Groups Default To Empty Object 35ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It 23ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 32ms
Request Payload.Disable Geoip Omitted Defaults To False 41ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 32ms
Request Lifecycle.No Flags Request On Init Alone 30ms
Request Lifecycle.No Flags Request On Normal Capture 51ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 29ms
Request Lifecycle.Mock Response Value Is Returned To Caller 31ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 21ms

Failures

request_payload.request_with_person_properties_device_id

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-67572fbe-e8e8-4cbb-a925-0c1ad65bc00f'

request_payload.flags_request_uses_v2_query_param

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-581af881-b0ec-4c2a-9fa7-324b43c45e8a'

request_payload.flags_request_hits_flags_path_not_decide

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-dc4eda4d-3756-4b91-a21c-fd94d43c310d'

request_payload.flags_request_omits_authorization_header

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-40af15fc-cda0-4a50-8322-7f1dbec17441'

request_payload.token_in_flags_body_matches_init

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-f6f6801e-4b0b-4e24-9073-380b8c11ba3c'

request_payload.groups_round_trip

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-69d92ece-1969-45de-ae2e-ea4ea3e2ad96'

request_payload.groups_default_to_empty_object

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-8f53abb4-ec4f-47a7-a6d2-7eb198f7071f'

request_payload.person_properties_distinct_id_auto_populated_when_caller_omits_it

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-2c8ed635-89f9-4b0d-b34e-d9a7077afa68'

request_payload.disable_geoip_false_propagates_as_geoip_disable_false

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-013810c4-8c12-48a0-950e-acdbff93b040'

request_payload.disable_geoip_omitted_defaults_to_false

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-3aa7426b-17f7-4e28-8c4e-490f0c95dad8'

request_payload.flag_keys_to_evaluate_contains_only_requested_key

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-b4419859-1843-4457-a7c3-9f8f9248765b'

request_lifecycle.two_flag_calls_produce_two_remote_requests

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-98253585-6638-4b45-b95f-dcb61e8172c0'

request_lifecycle.mock_response_value_is_returned_to_caller

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-59f7febe-ea30-4da9-9413-dd2a0501860b'

side_effect_events.get_feature_flag_captures_feature_flag_called_event

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag?test_id=t-c1af8e46-e464-4ae7-8e69-f03c90029e7c'

The standard library ships v0-mangled on Linux, so std frames demangle
with crate disambiguator hashes (std[b887e3750a86e3a0]::panicking::...)
that defeat every prefix check in the internal-frame filters - the panic
trace kept the boxed hook dispatch frame on top, failing the hook test
in CI. macOS resolves the same frames without hashes, which is why it
passed locally.

Normalize the hashes out at frame construction - they also vary per
rustc release, so sending them would break grouping on function names -
and strip the hook dispatch frame itself, which no prefix can match
(it names the hook argument type, PanicHookInfo, or PanicInfo before
the 1.82 rename).
@cat-ph cat-ph marked this pull request as ready for review June 12, 2026 13:59
@cat-ph cat-ph requested a review from a team as a code owner June 12, 2026 13:59
@cat-ph cat-ph requested review from a team, ablaszkiewicz and hpouillot June 12, 2026 13:59
Comment thread README.md Outdated
@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown

Reviews (2): Last reviewed commit: "fix: strip v0 crate disambiguators from ..." | Re-trigger Greptile

Per review on #130 (and the direction in #140), usage examples belong in
the posthog.com docs, not the README. Keep the feature blurb and a link;
the manual capture and panic hook docs move to posthog.com/docs.
@marandaneto

Copy link
Copy Markdown
Member

i think the hook should be installed during SDK init
eg
- init_global(...).with_panic_hook(...), or
- ClientOptions::capture_panics(true)

instead of a standalone public API - users would need to call install_panic_hook manually, they would need to find out etc

Comment thread src/client/mod.rs
Comment on lines +246 to +256
/// Timeout used by panic autocapture's synchronous best-effort request.
#[cfg(feature = "error-tracking")]
pub(crate) fn panic_capture_timeout_seconds(&self) -> u64 {
self.panic_capture_timeout_seconds
}

/// Project API key used by capture requests.
#[cfg_attr(not(feature = "error-tracking"), allow(dead_code))]
pub(crate) fn api_key(&self) -> &str {
&self.api_key
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tbh i dont like this approach
i think this sdk should have a runtime-independent transport layer, where the panic just enqueues new errors in the same way as we capture analytic events and then flushes right away if needed by calling flush
so not another HTTP code path and no specific timeouts for this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

should have a runtime-independent transport layer, where the panic just enqueues new errors

true, I kinda worked around not having this, I think our capture() has just a single event POST, no batching no queueing, no flush.. I think kinda similar to your point below, this was also to avoid the double panic and async capture, doing the manual request, but I don't particularly like it either 😬

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah now that i understand the double panic issue, i understand the reasoning
but i'd push back on doing this right
once we add public APIs, we cant remove them and have to support moving forward

Comment thread src/error_tracking.rs
// properties so they can't be overridden.
exception.write_into(&mut event, et_options)?;

send_panic_exception(options, event)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

instead of doing this, we should call the posthog instance (global?) with capture_exception(x) and let the instance enqueue and flush everything
a big gap by doing this here is that we are bypassing eg before_send
we can always invoke the before send wrapped by catch_unwind to avoid the double panic
if the SDK isnt designed to do that, i think we should fix that first

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah but I don't think catch_unwind helps unfortunately, I tried (asked the 🤖 to try) it:

use std::panic::{self, AssertUnwindSafe};

fn main() {
    let prev = panic::take_hook();
    panic::set_hook(Box::new(move |info| {
        // Simulate before_send / capture work that itself panics,
        // wrapped in catch_unwind
        let caught = panic::catch_unwind(AssertUnwindSafe(|| {
            println!("  [hook] running, about to panic inside hook...");
            panic!("panic raised INSIDE the hook (e.g. a before_send hook)");
        }));
        match caught {
            Ok(_) => println!("  [hook] catch_unwind returned Ok"),
            Err(_) => println!("  [hook] catch_unwind CAUGHT the inner panic -> safe"),
        }
        prev(info);
    }));

    println!("[main] triggering outer panic, caught by catch_unwind...");
    let outer = panic::catch_unwind(|| {
        panic!("outer panic");
    });
    println!("[main] outer catch_unwind returned: {:?} (if you see this, no abort)", outer.is_err());
}

=>

=== running (exit code is the verdict: 134 = SIGABRT) ===
[main] triggering outer panic, caught by catch_unwind...
  [hook] running, about to panic inside hook...
panicked at main.rs:10:13:

thread panicked while processing panic. aborting.
EXIT_CODE=134

what would help is running in in a separate thread (like we do with the HTTP request) where the panic count is 0 🤔

if the SDK isnt designed to do that, i think we should fix that first

100% agree with this though, yeah

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could try and put the before_send on the separate thread to avoid bypassing it & get it working, but maybe the better question is, do we want to do that (I'd do that either way though), or do we want to try and refactor the SDK first to do the transport work?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i'd 100% go with the transport work since this is beneficial for the SDK anyway
happy to pair up/review if needed
you can try to point an agent to https://github.com/PostHog/sdk-specs and another SDK as an example (eg python?) and get something quite quick

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

since its 0.x, we can also do a breaking change if needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks 💙 giving it a try then

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

put up a first version in #145 (a lot of code moving around 😅 ) & tested locally a bit, I'll stack / rebuild this one on top of that

Comment thread src/error_tracking.rs
Comment on lines +644 to +646
event.insert_prop("$exception_panic_file", location.file())?;
event.insert_prop("$exception_panic_line", location.line())?;
event.insert_prop("$exception_panic_column", location.column())?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not a Rust expert (at all) obviously but why do we have to append this as custom properties vs relying on just that capture_stacktrace() or adding panic_info.location() to the stack trace manually if capturing stack trace failed for whatever reason?

cat-ph added a commit to PostHog/posthog.com that referenced this pull request Jun 16, 2026
Panic autocapture is still in review (PostHog/posthog-rs#130) and
unreleased. Ship the manual error tracking docs now; the panic hook
section returns once it lands in a release.
cat-ph added a commit to PostHog/posthog.com that referenced this pull request Jun 17, 2026
* docs(rust): add Rust error tracking installation guide

Documents manual exception capture (capture_exception /
capture_exception_with), panic autocapture via install_panic_hook, and
stack trace configuration for the Rust SDK, moving the usage docs out of
the posthog-rs README per PostHog/posthog-rs#130 review. Adds the
installation page, the error tracking nav entry, and an error tracking
section on the Rust library page.

* docs(rust): include the API host in error tracking init examples

EU Cloud and self-hosted projects would otherwise send captures to the
default US endpoint; sibling installation pages all pass the host.

* docs(rust): drop panic autocapture until it ships

Panic autocapture is still in review (PostHog/posthog-rs#130) and
unreleased. Ship the manual error tracking docs now; the panic hook
section returns once it lands in a release.
@marandaneto marandaneto marked this pull request as draft June 19, 2026 06:21
@marandaneto

Copy link
Copy Markdown
Member

moving to draft until we get #145 so its out of our review filters

@cat-ph cat-ph mentioned this pull request Jun 25, 2026
@cat-ph

cat-ph commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

closing in favor of #157 - I rewrote a lot of the logic there to use the new transport so just using a new one instead.

@cat-ph cat-ph closed this Jun 25, 2026
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