Skip to content

Conversation

@ksinder
Copy link
Contributor

@ksinder ksinder commented Aug 27, 2025

This PR adds a few small fixes to our planned upcoming release of v5 of the SDK:

  • (Re-)add the mistakenly removed ExternalPageIconRequest and CustomEmojiPageIconRequest types under PageIconRequest
  • Add first-class support of the new optional additional_data field in public API error responses to APIResponseError
  • Commit some missing files for examples/ builds, e.g. examples/generate-random-data/tsconfig.json, that I forgot to commit up in [v5] Sync API shape changes for data_source parents and search filter #602
@ksinder ksinder requested review from Copilot, mquan and nsparikh August 27, 2025 23:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds missing page icon types and implements error handling improvements for the v5 SDK release. The changes focus on restoring previously removed page icon types and enhancing API error responses with additional metadata.

  • Re-adds ExternalPageIconRequest and CustomEmojiPageIconRequest types that were mistakenly removed
  • Implements support for the new additional_data field in API error responses
  • Commits missing build configuration files for examples

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/api-endpoints.ts Restores missing page icon types and adds new type definitions
src/errors.ts Adds support for additional_data field in API error handling
test/Client.test.ts Adds test coverage for the new error response format
examples/generate-random-data/tsconfig.json Adds missing TypeScript configuration file
examples/database-email-update/tsconfig.tsbuildinfo Adds TypeScript build info file

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

...parsed,
code: parsed["code"],
message: parsed["message"],
additional_data: parsed["additional_data"] as AdditionalData | undefined,
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

Using a type assertion as AdditionalData | undefined bypasses type checking and could lead to runtime errors if the API returns additional_data with unexpected structure. Consider implementing proper validation or using a more permissive type definition.

Suggested change
additional_data: parsed["additional_data"] as AdditionalData | undefined,
additional_data: isAdditionalData(parsed["additional_data"]) ? parsed["additional_data"] : undefined,
Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't really think of a good way to do a runtime check of the keys of an object being strings in JS :/ I feel like this is a necessary evil here (at the boundary of the web API call interface) and we kind of have to do some level of casting or ugly code and I chose the casting 😅)

nsparikh
nsparikh previously approved these changes Aug 27, 2025
Copy link

@nsparikh nsparikh left a comment

Choose a reason for hiding this comment

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

yay!

@ksinder ksinder merged commit 178b3c0 into main Aug 28, 2025
11 checks passed
@ksinder ksinder deleted the ksinder-data-source-updates branch August 28, 2025 00:00
tsinglinrain added a commit to tsinglinrain/notion-sdk-py that referenced this pull request Sep 13, 2025
ramnes pushed a commit to ramnes/notion-sdk-py that referenced this pull request Oct 23, 2025
* - Added `DataSourcesEndpoint` methods - Refactored `DatabasesEndpoint` methods (note: no pytest tests included) * - Synced with commit `698cd6ca37cdc2d0757ff40b2fed492a774344c5` from `notion-sdk-js`: in `DataSourcesEndpoint.update`, added `"parent"` to the `body` parameter * - Updated to align with commit `c1fc444b556dbc7b8a9af1abd3b1447211dfb538` from `notion-sdk-js`: added support for `is_locked` in update methods of page and database APIs * - Updated response handling to include the new `data_source` field: `additional_data` (see makenotion/notion-sdk-js#603) * - Kept the current VCR version - Updated `tests\cassettes` to change `notion-version: - '2025-09-03'` - Modified `tests\conftest.py` for format compatibility * - Upgraded `vcrpy` from `<6` to `vcrpy==6.0.2` - Added `"is_locked"` to the `body` of the `update` function in `DatabasesEndpoint` * ruff format * ruff format * - Added type annotations to ensure code consistency and clarity - Hardcoded `page_id` * - Temporarily removed Python 3.7 version testing - Normalize code formatting * Temporarily removed Python 3.7 version testing(I missed submitting last time) * - Added `request_id` field - Improved tests for `request_id` and `additional_data` - Enhanced `_parse_response` to handle `additional_data` and `request_id` - Improved initialization tests for `APIResponseError` * - Kept only data source and database update changes - Removed mentions of Python 3.7 support from `README.md` and other docs - Updated minimum supported Python version to 3.8 * - Updated `page_id` retrieval path in `tests\conftest.py` - Set `"decode_compressed_response"` back to `True` to ensure complete content in `tests\cassettes` - Fixed typo: renamed `test_file_uploads_list_with_stat_cursor` to `test_file_uploads_list_with_start_cursor` * - Added `data_source_id` in `tests\conftest.py` to reduce redundant test code in `tests\test_endpoints.py`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants