wip(native): sync WER data#1760
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
jpnurmi
force-pushed
the
jpnurmi/feat/native/wer-metadata
branch
from
c9a7b1a to
c218239
Compare
jpnurmi
changed the title
jpnurmi
force-pushed
the
jpnurmi/feat/native/wer-metadata
branch
from
c218239 to
b503ec0
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Pre-generate the native crash event ID and register it as WER custom metadata so the crash daemon can identify the matching WERInternalMetadata.xml. Store the WER report ID provided by sentry-wer.dll in the shared crash context and add a wer context with the report ID and app-provided WER custom metadata.
The crash handler's WER path returned EXCEPTION_CONTINUE_SEARCH without claiming the crash state or signaling the daemon, relying entirely on the sentry-wer DLL callback to do so via WER. On CI runners WER may not always invoke the registered callback, leaving the daemon waiting indefinitely. Fix by having the crash handler always claim the crash state and notify the daemon before returning, and having the sentry-wer DLL write the WER report ID and signal even when the crash handler already claimed the crash.
When the crash handler claims the crash in the WER path and notifies the daemon, it must set exception_pointers to NULL so the daemon uses the already-copied exception record/context from shared memory with ClientPointers=FALSE. Otherwise MiniDumpWriteDump tries to read from the crashed process's address space (ClientPointers=TRUE), but the process is already terminated because the crash handler returned EXCEPTION_CONTINUE_SEARCH without waiting.
jpnurmi
force-pushed
the
jpnurmi/feat/native/wer-metadata
branch
from
e7a5ac6 to
985abed
Compare
jpnurmi
changed the title
|
|
||
| from ast import pattern | ||
| import os | ||
| from pydoc import text |
There was a problem hiding this comment.
Stray unused test imports
Low Severity
The integration test file adds from ast import pattern and from pydoc import text, which are not used anywhere in the module. The WER helper uses a local variable named text, not the pydoc import. These look like accidental editor additions unrelated to the WER sync work.
Reviewed by Cursor Bugbot for commit 678581b. Configure here.
This reverts commit 985abed.
| envelope = Envelope.deserialize(httpserver.log[0][0].get_data()) | ||
| assert_native_crash(envelope, exception_code=0xC0000409) | ||
|
|
||
| event = envelope.get_event() |
There was a problem hiding this comment.
Wrong exception code for fastfail
Low Severity
test_native_wer always asserts exception code 0xC0000409 while also parametrizing crash_arg with fastfail, but __fastfail crashes use STATUS_FAIL_FAST_EXCEPTION (0xC0000602), so the fastfail case fails the mechanism check.
Reviewed by Cursor Bugbot for commit 9dc23d7. Configure here.
| event_json = new_event_json; | ||
| event_size = new_event_size; | ||
| } | ||
| } |
There was a problem hiding this comment.
WER context dropped on JSON failure
Low Severity
In minidump-only envelope construction, after add_wer_context mutates the parsed event, a failed sentry__value_to_json leaves the original on-disk JSON unchanged, so the uploaded envelope can omit the WER context even though enrichment succeeded in memory.
Reviewed by Cursor Bugbot for commit 9dc23d7. Configure here.
| sentry_value_decref(tags); | ||
| } | ||
|
|
||
| return event; |
There was a problem hiding this comment.
WER parser omits report_id
Medium Severity
parse_internal_metadata only copies ProcessMetadata into top-level tags and never builds contexts.wer.report_id from ReportInformation/Guid, but the new wer_metadata unit test expects that shape from sentry__wer_report_from_buffer.
Reviewed by Cursor Bugbot for commit b088def. Configure here.
| // the crash first, it already SetEvent'd via sentry__crash_ipc_notify, | ||
| // but a redundant SetEvent on an auto-reset event is harmless. | ||
| if (SetEvent(event)) { | ||
| claimed = TRUE; |
There was a problem hiding this comment.
WER claim omits process termination
High Severity
The WER callback still transitions shared crash state to CRASHED and sets ownership_claimed when it wins the claim, but this change removed the prior TerminateProcess (and wait-for-daemon) that ran after a successful claim, so fail-fast paths handled only by WER may never exit.
Reviewed by Cursor Bugbot for commit b088def. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a6f70e9. Configure here.
jpnurmi
force-pushed
the
jpnurmi/feat/native/wer-metadata
branch
from
a6f70e9 to
cfb5df9
Compare
|
Superceded by #1777 |
1 participant
App:
Context:
{ "contexts": { "wer": { "type": "wer", "report_id": "...", "metadata": { "foo": "ooo", "bar": "aaa", "baz": "zzz" } } } }Sentry:
