Skip to content

Commit 2fa9aa0

Browse files
jsaabelclaude
andcommitted
fix: handle unknown property types gracefully
Pages with unsupported property types (button, unique_id, place, etc.) now deserialize successfully as PageProperty.Unknown instead of failing with "Network error occurred". This ensures forward compatibility as Notion adds new property types, allowing the client to work with any Notion workspace regardless of which modern property types are used. Changes: - Add PageProperty.Unknown data class to store unsupported types - Implement PagePropertySerializer with fallback to Unknown - Preserve raw JSON in Unknown.rawContent for inspection - Add comprehensive unit tests (4 tests, 100% passing) - Add integration test for manual verification Testing: - All 503 unit tests passing (no regressions) - Integration tested with button, unique_id, and place properties - Verified fix in production use case (festival-scripts) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ffb1593 commit 2fa9aa0

File tree

5 files changed

+713
-1
lines changed

5 files changed

+713
-1
lines changed
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
# Session: Unsupported Property Types Investigation
2+
3+
**Date:** 2025-11-03
4+
**Focus:** Investigate and fix deserialization failures when pages contain unsupported property types
5+
6+
## Problem Statement
7+
8+
During production integration in `festival-scripts`, discovered that queries fail when pages contain unsupported property types (confirmed: button properties). The client throws a misleading "Network error occurred" message instead of gracefully handling unknown properties.
9+
10+
### Reproduction
11+
1. Query data source with standard properties → ✅ Works
12+
2. Add button property to any page → ❌ Fails with "Network error occurred"
13+
3. Remove button property → ✅ Works again
14+
15+
### Impact
16+
- **Severity:** High - blocks production use
17+
- Real workspaces commonly use button properties and modern features
18+
- Workaround (removing properties) not viable
19+
20+
### Confirmed Problematic Types
21+
- Button properties (confirmed)
22+
- Likely: AI properties, unique ID, verification, etc.
23+
24+
## Investigation Plan
25+
26+
1. **Find deserialization code:**
27+
- Locate where API responses convert to Page objects
28+
- Find property type mapping/parsing logic
29+
- Identify source of "Network error occurred" message
30+
31+
2. **Understand current implementation:**
32+
- How are property types defined?
33+
- Is there strict vs lenient deserialization?
34+
- What's the error handling flow?
35+
36+
3. **Design solution:**
37+
- Option A: Skip unknown properties (best for forward compatibility)
38+
- Option B: Generic "Unknown" property type with raw JSON preserved
39+
- Option C: At minimum, clear error messages
40+
41+
4. **Test with real data:**
42+
- User can create pages with button properties
43+
- Verify fix handles unknown types gracefully
44+
- Ensure no regression on existing property types
45+
46+
## Session Log
47+
48+
### Initial Investigation
49+
50+
**Root Cause Identified:**
51+
52+
1. **Problem location:** `PageProperty.kt` (sealed class with specific subtypes)
53+
- Uses `@Serializable` with `@SerialName` for each property type
54+
- No fallback for unknown types like "button", "unique_id", "verification", etc.
55+
56+
2. **Deserialization flow:**
57+
- API returns page with properties: `Map<String, PageProperty>`
58+
- kotlinx.serialization tries to deserialize each property based on "type" field
59+
- When encountering unknown type (e.g., "button"), no matching `@SerialName` exists
60+
- Throws `SerializationException`
61+
62+
3. **Error handling wrapping:**
63+
- `DataSourcesApi.kt:211-214` catches all `Exception` types (not just `NotionException`)
64+
- Wraps deserialization errors as `NotionException.NetworkError`
65+
- Results in misleading "Network error occurred" message
66+
67+
4. **Current serializer setup:**
68+
- NotionClient.kt:145 - Uses `ignoreUnknownKeys = true`
69+
- This only ignores unknown JSON keys, NOT unknown sealed class subtypes
70+
- Sealed classes need custom serializers for graceful fallback
71+
72+
### Solution Design
73+
74+
**Approach:** Custom serializer with fallback to Unknown type
75+
76+
**Pattern identified:** Existing custom serializers in codebase:
77+
- `ParentSerializer` - Uses `JsonContentPolymorphicSerializer`
78+
- `PageIconSerializer` - Same pattern
79+
- Both currently throw on unknown types, but we'll improve this
80+
81+
**Implementation plan:**
82+
1. Add `Unknown` property type to `PageProperty` sealed class
83+
- Stores raw JSON as `JsonElement`
84+
- Includes `id`, `type`, and `rawContent` fields
85+
86+
2. Create `PagePropertySerializer`
87+
- Extends `JsonContentPolymorphicSerializer<PageProperty>`
88+
- Maps known types to their serializers
89+
- Falls back to `Unknown` for unrecognized types
90+
91+
3. Register serializer with `@Serializable(with = PagePropertySerializer::class)`
92+
93+
4. Add unit tests
94+
- Test deserialization of button property
95+
- Test other unknown property types
96+
- Verify existing property types still work
97+
98+
**Benefits:**
99+
- Forward compatibility - New Notion property types won't break the client
100+
- Preserves data - Raw JSON available for inspection
101+
- Better UX - No cryptic "Network error" messages
102+
- Allows gradual addition of property type support
103+
104+
### Implementation Complete
105+
106+
**Files Modified:**
107+
1. `PageProperty.kt` - Added `PageProperty.Unknown` data class
108+
2. `PagePropertySerializer.kt` - New custom serializer with fallback handling
109+
3. `PagePropertyUnknownTypeTest.kt` - 4 unit tests (100% passing)
110+
4. `UnknownPropertyTypesIntegrationTest.kt` - Manual verification integration test
111+
112+
**Test Results:**
113+
- Unit tests: 4 new tests, 100% passing
114+
- Full test suite: 503 tests, 100% passing (no regressions)
115+
- Integration test: ✅ Verified with real button, unique_id, and place properties
116+
- Real-world verification: ✅ Fixed original issue in festival-scripts
117+
118+
**Integration Test Results:**
119+
- Tested with 3 unknown types: button, unique_id, place
120+
- Tested with 13 supported types in same page
121+
- All unknown types correctly deserialized as `PageProperty.Unknown`
122+
- All supported types continue to work normally
123+
124+
**Real-World Verification:**
125+
- Published to Maven Local (0.1.0)
126+
- Tested in festival-scripts project
127+
- ✅ Original failing query now succeeds
128+
- ✅ Pages with button properties load successfully
129+
- ✅ Can access supported properties normally
130+
131+
## Outcome
132+
133+
**Issue Resolved**: Pages with unsupported property types now deserialize successfully as `PageProperty.Unknown`
134+
135+
**Production Ready**: Client can handle any Notion workspace regardless of property types used
136+
137+
**Tests Passing**: 100% test success rate (503 unit tests + integration test)
138+
139+
**Real-World Verified**: Fixed original issue in festival-scripts production use case
140+
141+
## Next Steps / Future Considerations
142+
143+
### Candidate for Proper Implementation: Unique ID
144+
145+
The `unique_id` property type has high utility and should be considered for proper implementation:
146+
147+
**Why it's valuable:**
148+
- Auto-incrementing numeric IDs with customizable prefixes
149+
- Common for tracking items, tickets, invoices, etc.
150+
- Structured data that's useful to access programmatically
151+
152+
**Example structure:**
153+
```json
154+
{
155+
"id": "prop-id",
156+
"type": "unique_id",
157+
"unique_id": {
158+
"prefix": "TASK",
159+
"number": 123
160+
}
161+
}
162+
```
163+
164+
**Potential implementation:**
165+
```kotlin
166+
@Serializable
167+
@SerialName("unique_id")
168+
data class UniqueId(
169+
@SerialName("id") override val id: String,
170+
@SerialName("type") override val type: String,
171+
@SerialName("unique_id") val uniqueId: UniqueIdValue?,
172+
) : PageProperty()
173+
174+
@Serializable
175+
data class UniqueIdValue(
176+
@SerialName("prefix") val prefix: String?,
177+
@SerialName("number") val number: Int,
178+
)
179+
```
180+
181+
**Other unknown types** (button, place) have limited utility and can remain as `Unknown`:
182+
- **Button**: Just triggers actions, no data to retrieve
183+
- **Place**: Location data, but without structured access, raw JSON is sufficient
184+
185+
This could be tackled in a future session with fresh context.

src/main/kotlin/it/saabel/kotlinnotionclient/models/pages/PageProperty.kt

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import kotlinx.datetime.Instant
88
import kotlinx.datetime.toLocalDateTime
99
import kotlinx.serialization.SerialName
1010
import kotlinx.serialization.Serializable
11+
import kotlinx.serialization.json.JsonElement
1112

1213
/**
1314
* Typed models for page properties as returned from the Notion API.
@@ -40,7 +41,7 @@ import kotlinx.serialization.Serializable
4041
* val score = page.getNumberProperty("Score") ?: 0.0
4142
* ```
4243
*/
43-
@Serializable
44+
@Serializable(with = PagePropertySerializer::class)
4445
sealed class PageProperty {
4546
abstract val id: String
4647
abstract val type: String
@@ -211,6 +212,23 @@ sealed class PageProperty {
211212
@SerialName("type") override val type: String,
212213
@SerialName("last_edited_by") val lastEditedBy: User,
213214
) : PageProperty()
215+
216+
/**
217+
* Represents an unsupported or unknown property type.
218+
*
219+
* This type is used as a fallback when the Notion API returns a property type
220+
* that isn't explicitly supported by the client (e.g., "button", "unique_id",
221+
* "verification", etc.). This ensures forward compatibility as Notion adds new
222+
* property types.
223+
*
224+
* The raw JSON is preserved so users can inspect it or handle it manually.
225+
*/
226+
@Serializable
227+
data class Unknown(
228+
@SerialName("id") override val id: String,
229+
@SerialName("type") override val type: String,
230+
val rawContent: JsonElement,
231+
) : PageProperty()
214232
}
215233

216234
/**
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package it.saabel.kotlinnotionclient.models.pages
2+
3+
import kotlinx.serialization.DeserializationStrategy
4+
import kotlinx.serialization.KSerializer
5+
import kotlinx.serialization.SerializationException
6+
import kotlinx.serialization.descriptors.SerialDescriptor
7+
import kotlinx.serialization.descriptors.buildClassSerialDescriptor
8+
import kotlinx.serialization.encoding.Decoder
9+
import kotlinx.serialization.encoding.Encoder
10+
import kotlinx.serialization.json.JsonContentPolymorphicSerializer
11+
import kotlinx.serialization.json.JsonDecoder
12+
import kotlinx.serialization.json.JsonElement
13+
import kotlinx.serialization.json.jsonObject
14+
import kotlinx.serialization.json.jsonPrimitive
15+
16+
/**
17+
* Custom serializer for [PageProperty] sealed class.
18+
*
19+
* This serializer handles the polymorphic serialization and deserialization of PageProperty objects
20+
* based on the "type" discriminator field in the JSON, maintaining compatibility with the
21+
* Notion API's response format.
22+
*
23+
* Unlike other serializers in the codebase, this one gracefully handles unknown property types
24+
* by deserializing them as [PageProperty.Unknown], ensuring forward compatibility as Notion
25+
* adds new property types (e.g., "button", "unique_id", "verification", etc.).
26+
*
27+
* ## Supported Property Types
28+
* - title, rich_text, number, checkbox, url, email, phone_number
29+
* - select, multi_select, status
30+
* - date, people, files
31+
* - relation, formula, rollup
32+
* - created_time, last_edited_time, created_by, last_edited_by
33+
*
34+
* ## Unknown Property Types
35+
* Any property type not in the list above will be deserialized as [PageProperty.Unknown],
36+
* with the raw JSON preserved in the `rawContent` field for inspection or manual handling.
37+
*/
38+
object PagePropertySerializer : KSerializer<PageProperty> {
39+
override val descriptor: SerialDescriptor = buildClassSerialDescriptor("PageProperty")
40+
41+
override fun deserialize(decoder: Decoder): PageProperty {
42+
require(decoder is JsonDecoder) { "PagePropertySerializer can only deserialize JSON" }
43+
44+
val element = decoder.decodeJsonElement()
45+
val jsonObject = element.jsonObject
46+
47+
val type =
48+
jsonObject["type"]?.jsonPrimitive?.content
49+
?: throw SerializationException("Missing 'type' field in PageProperty JSON")
50+
51+
val id =
52+
jsonObject["id"]?.jsonPrimitive?.content
53+
?: throw SerializationException("Missing 'id' field in PageProperty JSON")
54+
55+
return when (type) {
56+
"title" -> decoder.json.decodeFromJsonElement(PageProperty.Title.serializer(), element)
57+
"rich_text" -> decoder.json.decodeFromJsonElement(PageProperty.RichTextProperty.serializer(), element)
58+
"number" -> decoder.json.decodeFromJsonElement(PageProperty.Number.serializer(), element)
59+
"checkbox" -> decoder.json.decodeFromJsonElement(PageProperty.Checkbox.serializer(), element)
60+
"url" -> decoder.json.decodeFromJsonElement(PageProperty.Url.serializer(), element)
61+
"email" -> decoder.json.decodeFromJsonElement(PageProperty.Email.serializer(), element)
62+
"phone_number" -> decoder.json.decodeFromJsonElement(PageProperty.PhoneNumber.serializer(), element)
63+
"select" -> decoder.json.decodeFromJsonElement(PageProperty.Select.serializer(), element)
64+
"multi_select" -> decoder.json.decodeFromJsonElement(PageProperty.MultiSelect.serializer(), element)
65+
"status" -> decoder.json.decodeFromJsonElement(PageProperty.Status.serializer(), element)
66+
"date" -> decoder.json.decodeFromJsonElement(PageProperty.Date.serializer(), element)
67+
"people" -> decoder.json.decodeFromJsonElement(PageProperty.People.serializer(), element)
68+
"files" -> decoder.json.decodeFromJsonElement(PageProperty.Files.serializer(), element)
69+
"relation" -> decoder.json.decodeFromJsonElement(PageProperty.Relation.serializer(), element)
70+
"formula" -> decoder.json.decodeFromJsonElement(PageProperty.Formula.serializer(), element)
71+
"rollup" -> decoder.json.decodeFromJsonElement(PageProperty.Rollup.serializer(), element)
72+
"created_time" -> decoder.json.decodeFromJsonElement(PageProperty.CreatedTime.serializer(), element)
73+
"last_edited_time" -> decoder.json.decodeFromJsonElement(PageProperty.LastEditedTime.serializer(), element)
74+
"created_by" -> decoder.json.decodeFromJsonElement(PageProperty.CreatedBy.serializer(), element)
75+
"last_edited_by" -> decoder.json.decodeFromJsonElement(PageProperty.LastEditedBy.serializer(), element)
76+
else -> {
77+
// Fallback to Unknown for unsupported property types
78+
// Manually construct Unknown with the raw JSON element
79+
PageProperty.Unknown(
80+
id = id,
81+
type = type,
82+
rawContent = element,
83+
)
84+
}
85+
}
86+
}
87+
88+
override fun serialize(
89+
encoder: Encoder,
90+
value: PageProperty,
91+
) {
92+
// For serialization, delegate to the appropriate serializer
93+
when (value) {
94+
is PageProperty.Title -> encoder.encodeSerializableValue(PageProperty.Title.serializer(), value)
95+
is PageProperty.RichTextProperty ->
96+
encoder.encodeSerializableValue(
97+
PageProperty.RichTextProperty.serializer(),
98+
value,
99+
)
100+
is PageProperty.Number -> encoder.encodeSerializableValue(PageProperty.Number.serializer(), value)
101+
is PageProperty.Checkbox -> encoder.encodeSerializableValue(PageProperty.Checkbox.serializer(), value)
102+
is PageProperty.Url -> encoder.encodeSerializableValue(PageProperty.Url.serializer(), value)
103+
is PageProperty.Email -> encoder.encodeSerializableValue(PageProperty.Email.serializer(), value)
104+
is PageProperty.PhoneNumber -> encoder.encodeSerializableValue(PageProperty.PhoneNumber.serializer(), value)
105+
is PageProperty.Select -> encoder.encodeSerializableValue(PageProperty.Select.serializer(), value)
106+
is PageProperty.MultiSelect -> encoder.encodeSerializableValue(PageProperty.MultiSelect.serializer(), value)
107+
is PageProperty.Status -> encoder.encodeSerializableValue(PageProperty.Status.serializer(), value)
108+
is PageProperty.Date -> encoder.encodeSerializableValue(PageProperty.Date.serializer(), value)
109+
is PageProperty.People -> encoder.encodeSerializableValue(PageProperty.People.serializer(), value)
110+
is PageProperty.Files -> encoder.encodeSerializableValue(PageProperty.Files.serializer(), value)
111+
is PageProperty.Relation -> encoder.encodeSerializableValue(PageProperty.Relation.serializer(), value)
112+
is PageProperty.Formula -> encoder.encodeSerializableValue(PageProperty.Formula.serializer(), value)
113+
is PageProperty.Rollup -> encoder.encodeSerializableValue(PageProperty.Rollup.serializer(), value)
114+
is PageProperty.CreatedTime -> encoder.encodeSerializableValue(PageProperty.CreatedTime.serializer(), value)
115+
is PageProperty.LastEditedTime ->
116+
encoder.encodeSerializableValue(
117+
PageProperty.LastEditedTime.serializer(),
118+
value,
119+
)
120+
is PageProperty.CreatedBy -> encoder.encodeSerializableValue(PageProperty.CreatedBy.serializer(), value)
121+
is PageProperty.LastEditedBy ->
122+
encoder.encodeSerializableValue(
123+
PageProperty.LastEditedBy.serializer(),
124+
value,
125+
)
126+
is PageProperty.Unknown -> encoder.encodeSerializableValue(PageProperty.Unknown.serializer(), value)
127+
}
128+
}
129+
}

0 commit comments

Comments
 (0)