Skip to content

fix: be a more tolerant reader of decide response #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

pauldambra
Copy link
Member

When starting the example app pointing at my local instance (but it would have been true of most any instance of PostHog)

The application was exploding because the client was trying to deserialize the sessionRecording property of the decide response into a boolean but it is only boolean when False otherrwise it's an object.

Since the web SDK is the closest we have to a reference SDK it is relatively common for us to both implicitly and explicitly rely on the type of things being boolean | BlahConfig for example https://github.com/PostHog/posthog-js/blob/e0c4ba4f3610c17480b8b250c9b3a11cc160cc3e/src/types.ts#L532

Since properties that are boolean today could change to be boolean | BlahConfig in future I opted to remove properties that looked mostly front-end specific. To avoid them exploding

I don't think we can support web sdk things like heatmaps and session recording in this SDK

And can add them back when we need them if we can support them here

@pauldambra pauldambra requested a review from haacked as a code owner January 16, 2025 18:56
@pauldambra
Copy link
Member Author

An unhandled exception occurred while processing the request.
InvalidOperationException: Cannot get the value of a token type 'StartObject' as a boolean.
System.Text.Json.ThrowHelper.ThrowInvalidOperationException_ExpectedBoolean(JsonTokenType tokenType)

JsonException: The JSON value could not be converted to System.Boolean. Path: $.sessionRecording | LineNumber: 0 | BytePositionInLine: 3293.
System.Text.Json.ThrowHelper.ReThrowWithPath(ref ReadStack state, ref Utf8JsonReader reader, Exception ex)

Stack Query Cookies Headers Routing
InvalidOperationException: Cannot get the value of a token type 'StartObject' as a boolean.
System.Text.Json.ThrowHelper.ThrowInvalidOperationException_ExpectedBoolean(JsonTokenType tokenType)
System.Text.Json.Utf8JsonReader.GetBoolean()
System.Text.Json.Serialization.Metadata.JsonPropertyInfo<T>.ReadJsonAndSetMember(object obj, ref ReadStack state, ref Utf8JsonReader reader)
System.Text.Json.Serialization.Converters.ObjectDefaultConverter<T>.OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out T value)
System.Text.Json.Serialization.JsonConverter<T>.TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out T value, out bool isPopulatedValue)
System.Text.Json.Serialization.JsonConverter<T>.ReadCore(ref Utf8JsonReader reader, out T value, JsonSerializerOptions options, ref ReadStack state)

Show raw exception details
JsonException: The JSON value could not be converted to System.Boolean. Path: $.sessionRecording | LineNumber: 0 | BytePositionInLine: 3293.

unformatted stack of the error for posterity

@haacked
Copy link
Collaborator

haacked commented Jan 16, 2025

Thanks!

Copy link
Collaborator

@haacked haacked left a comment

Choose a reason for hiding this comment

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

I support removing items until we actually need them.

@haacked haacked merged commit 3435eee into main Jan 16, 2025
4 checks passed
@haacked haacked deleted the fix/pd/fewer-decide-response-props branch January 16, 2025 21:52
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