fix(apple-crash-report): Updating deprecated debug_image fields by wmak · Pull Request #15749 · getsentry/sentry

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak · 2019-11-21T16:47:44Z

This uses .get and defaults to unknown on an image name,
preventing a key error since the name is an optional field and not
guaranteed to be part of the debug_image object
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#macho-images
Fixes SENTRY-C55

- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

dashed · 2019-11-21T19:48:51Z

 offset = ""
 if frame.get("image_addr") is not None and (
 not self.symbolicated
- or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING


@wmak I recently saw these PRs, which may be thee reason why this particular style of value coercion was used.
#15549
#15602
/cc @untitaker

yeah pretty sure that won't work

Event normalization replaces invalid values with an explicit None in many cases. The default value in get is only applied if the key is missing completely, so it would be safer to leave the or here.

Ah whoops sorry, didn't realize that. I'll flip these back 👍

markstory · 2019-11-21T20:09:40Z

 frame["lineno"],
 )
- symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file)
+ symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file)


Could this key be a falsey value ever? You might still need the or case to handle that scenario.

All these could be None explicitly, so the original implementation was safer.

untitaker

If you're serious about changing all the get() or calls I would definetly only try that in a separate deploy. The change about name looks good

jan-auer · 2019-11-21T20:30:40Z

 self.context["device"]["arch"],
 (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(),
- debug_image["name"],
+ debug_image.get("name", NATIVE_UNKNOWN_STRING),


Wait, "name" is long gone and now normalized into "code_file" which should be used here, instead. I'm afraid tests for this are very outdated :(

The schema is documented here:
https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/
Schema implementation is here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L125
Normalization of legacy types is implemented here:
https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/store/legacy.rs#L19

Not quite, AppleDebugImage is deprecated and normalized into NativeDebugImage, which only has a "code_file" (see my last link in the previous comment). Docs are definitely up to date.

(ignore last comment)

Here is a somewhat realistic snapshot of a current debug image payload, unfortunately in yaml this time:
sentry/tests/symbolicator/snapshots/SymbolicatorMinidumpIntegrationTest/test_full_minidump.pysnap
Lines 16 to 30 in e0d2322
- arch: x86
code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
code_id: 5ab380779000
debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
debug_status: found
features:
has_debug_info: true
has_sources: false
has_symbols: true
has_unwind_info: true
image_addr: '0x2a0000'
image_size: 36864
type: pe
unwind_status: found

👍 Thanks! I'll also try to update these tests to be closer here too.

@untitaker @jan-auer Actually before I keep making changes.
is code_file optional? I think I might be misunderstanding the process here.
I see required here: https://github.com/getsentry/semaphore/blob/2e39b6dd6cf64a15d13800f68af2f86d0a8bcaf9/general/src/protocol/debugmeta.rs#L132
But optional here: https://docs.sentry.io/development/sdk-dev/event-payloads/debugmeta/#debug-images
Is it optional from the user side, but during processing it becomes required (and defaulted I assume)?

…mages" This reverts commit ec69242.

- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

jan-auer · 2019-11-26T15:31:10Z

 hex(image_addr),
 hex(image_addr + debug_image["image_size"] - 1),
- image_name(debug_image["name"]),
+ image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING),


dashed

Looks good to me 👍

- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

fix(apple-crash-report): Name is an optional field in debug images
ec69242
- This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object

wmak added the Type: Bug label Nov 21, 2019

wmak requested a review from a teamNovember 21, 2019 16:47

dashed requested review from HazAT and untitaker November 21, 2019 18:27

dashed reviewed Nov 21, 2019
View reviewed changes

markstory reviewed Nov 21, 2019
View reviewed changes

untitaker requested a review from jan-auer November 21, 2019 20:24

untitaker reviewed Nov 21, 2019
View reviewed changes

jan-auer reviewed Nov 21, 2019
View reviewed changes

wmak added 2 commits November 21, 2019 17:10

Revert "fix(apple-crash-report): Name is an optional field in debug i…
4a97505
…mages" This reverts commit ec69242.

fix(apple-crash-report): Name was deprecated for code file
cea9b87
- Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern.

dashed requested review from dashed, jan-auer, markstory and untitaker November 22, 2019 07:23

untitaker requested changes Nov 22, 2019
View reviewed changes

Comment threadtests/sentry/lang/native/test_applecrashreport.py

wmak changed the title ~~fix(apple-crash-report): Name is an optional field in debug images~~fix(apple-crash-report): Updating deprecated debug_image fieldsNov 22, 2019

ref: remove the other deprecated fields
f2a6922

wmak force-pushed the fix/brittle-debug-image branch from aaffc90 to f2a6922Compare November 25, 2019 23:26

wmak requested a review from untitaker November 25, 2019 23:27

untitaker approved these changes Nov 26, 2019
View reviewed changes

jan-auer approved these changes Nov 26, 2019
View reviewed changes

dashed approved these changes Nov 26, 2019
View reviewed changes

wmak merged commit a8872ce into masterNov 26, 2019

wmak deleted the fix/brittle-debug-image branch November 26, 2019 16:15

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
ec7c73f
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

wmak mentioned this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id #15835
Merged

wmak added a commit that referenced this pull request Nov 26, 2019
fix(apple-crash-report): id field was deprecated for debug_id
105af39
- As part of #15749 I missed that the `id` and `uuid` fields were deprecated as well. This updates that and switches to the new `debug_id` field

github-actionsBot locked and limited conversation to collaborators Dec 19, 2020

	- arch: x86
	code_file: C:\projects\breakpad-tools\windows\Release\crash.exe
	code_id: 5ab380779000
	debug_file: C:\projects\breakpad-tools\windows\Release\crash.pdb
	debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936-1
	debug_status: found
	features:
	has_debug_info: true
	has_sources: false
	has_symbols: true
	has_unwind_info: true
	image_addr: '0x2a0000'
	image_size: 36864
	type: pe
	unwind_status: found

Uh oh!

Uh oh!

Conversation

wmak commented Nov 21, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

untitaker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jan-auerNov 21, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dashed left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

wmak commented Nov 21, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

untitaker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jan-auerNov 21, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wmak commented Nov 21, 2019•
edited
Loading

jan-auerNov 21, 2019•
edited
Loading

wmak commented Nov 21, 2019•
edited
Loading

jan-auerNov 21, 2019•
edited
Loading

wmak commented Nov 21, 2019•
edited
Loading