Skip to content

fix(logs): Allow null custom attributes without debug warning#1552

Merged
limbonaut merged 4 commits intomasterfrom
fix/allow-null-log-attributes
Mar 4, 2026
Merged

fix(logs): Allow null custom attributes without debug warning#1552
limbonaut merged 4 commits intomasterfrom
fix/allow-null-log-attributes

Conversation

@limbonaut
Copy link
Collaborator

Passing null as custom attributes to log functions prints a spurious debug message ("Discarded custom attributes on log: non-object sentry_value_t passed in"). Null is a valid way to indicate "no custom attributes" — the metrics path in construct_metric() already handles this correctly.

Came up while working on sentry-godot metrics support. The Godot SDK has a shared dictionary_to_attributes() helper used by both logs and metrics. Ideally it returns null for empty dictionaries to avoid an unnecessary allocation, but the log path warned about it.

Null is a valid way to pass no custom attributes, consistent with how
metrics already handles this in construct_metric().

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Collaborator

@jpnurmi jpnurmi left a comment

Choose a reason for hiding this comment

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

Thanks! Looks like a miss. Even the docs suggest passing sentry_value_new_null().

* You can pass `sentry_value_new_null()` to logs which don't need attributes.

@limbonaut limbonaut force-pushed the fix/allow-null-log-attributes branch from 35ad0f7 to 0876c01 Compare March 4, 2026 15:50
@limbonaut
Copy link
Collaborator Author

Refactored to use switch statement for clarity.

@jpnurmi
Copy link
Collaborator

jpnurmi commented Mar 4, 2026

Refactored to use switch statement for clarity.

Not sure why/how/where the flag comes from, but the Windows ClangCL configuration builds with -Wswitch-enum:

D:\a\sentry-native\sentry-native\src\sentry_logs.c(256,21): error : 7 enumeration values not explicitly handled in switch: 'SENTRY_VALUE_TYPE_BOOL', 'SENTRY_VALUE_TYPE_INT32', 'SENTRY_VALUE_TYPE_INT64'... [-Werror,-Wswitch-enum] [C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\cmake0\sentry.vcxproj]

@limbonaut limbonaut force-pushed the fix/allow-null-log-attributes branch from 0876c01 to 95c4816 Compare March 4, 2026 16:40
@limbonaut
Copy link
Collaborator Author

limbonaut commented Mar 4, 2026

I see, hmm. Looks like it wants all flags to be explicitly handled? default doesn't cut it. I rolled back to if-statements.

@limbonaut
Copy link
Collaborator Author

Found this: llvm/llvm-project#148856

@jpnurmi
Copy link
Collaborator

jpnurmi commented Mar 4, 2026

Claude suggests using /clang:-Wall instead of -Wall with clang-cl. Unlike regular clang where -Wall is a curated subset, clang-cl's -Wall means -Weverything which includes -Wswitch-enum. 😵

Edit: I was just curious, but didn't mean we'd start messing with the flags in this PR, by the way. I'm happy with the original else-if proposal.

@limbonaut limbonaut merged commit de168f1 into master Mar 4, 2026
80 of 81 checks passed
@limbonaut limbonaut deleted the fix/allow-null-log-attributes branch March 4, 2026 20:55
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.

2 participants