Page size and error handling#12
Conversation
… error handling tests
…berryProducts/md-notion into page-size-and-error-handling
There was a problem hiding this comment.
Pull request overview
This pull request introduces error handling improvements and pagination support for the Notion SDK. The main changes include a custom NotionApiException for Notion API errors with helper methods for common error types, and automatic pagination for fetching collections larger than 100 items (the Notion API's per-request limit).
Key changes:
- New
NotionApiExceptionclass with methods likeisRateLimited(),isUnauthorized(),isRetryable(), etc. - Automatic pagination in
Actions::getBlockChildren()andActions::queryDataSource()when requested items exceed 100 - Page size configuration support via
default_page_sizeconfig parameter passed throughPageReaderandDatabaseReader
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SDK/Exceptions/NotionApiException.php | New custom exception class for Notion API errors with helper methods |
| src/SDK/Notion.php | Added AlwaysThrowOnErrors trait and getRequestException() method to throw NotionApiException |
| src/SDK/Resource/Actions.php | Added auto-pagination logic for getBlockChildren() and queryDataSource() with conditional return types |
| src/SDK/Requests/Actions/BlockChildren.php | Changed pageSize parameter from string to int, added startCursor parameter |
| src/SDK/Requests/Actions/QueryDataSource.php | Added HasBody interface, pageSize and startCursor parameters with defaultBody() implementation |
| src/Services/PageReader.php | Added pageSize parameter with config default fallback and handling for dual return types |
| src/Services/DatabaseReader.php | Added pageSize parameter with config default fallback and handling for dual return types |
| tests/SDK/NotionApiExceptionTest.php | Comprehensive tests for exception handling and helper methods |
| tests/Services/PageSizeTest.php | Tests for page size configuration with PageReader and DatabaseReader |
| tests/SDK/Requests/BlockChildrenRequestTest.php | Updated test to use integer pageSize instead of string |
| examples/page-size-test.php | Manual test script demonstrating page size configuration and auto-pagination |
| examples/error-handling-test.php | Manual test script demonstrating error handling with NotionApiException |
Comments suppressed due to low confidence (1)
src/Services/DatabaseReader.php:63
- The variable
$queryDatamay be undefined when reaching line 62 if there are no data sources or if all data sources lack an 'id'. This would cause a PHP warning/error when accessing$queryData['results'].
Consider initializing $queryData before the foreach loop or wrapping lines 60-69 in a condition that checks if $queryData is defined.
// Resolve database items as collection of Page objects
$items = collect();
$results = $queryData['results'] ?? [];
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
src/Services/DatabaseReader.php:62
- The variable
$tableContentmay be undefined at this line if the database has no data sources or if the foreach loop doesn't execute. This could cause a runtime error. While line 58 has a fallback ($tableContent ?? ''), the variable$queryDataused on line 62 will also be undefined in the same scenario, which will cause an error. Both variables should be initialized before the data sources loop.
$database->setTableContent($tableContent ?? '');
// Resolve database items as collection of Page objects
$items = collect();
$results = $queryData['results'] ?? [];
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request introduces robust error handling for the Notion SDK and adds support for automatic pagination when fetching large collections of data. The main highlights include a new custom exception for Notion API errors, improved handling of paginated requests, and comprehensive manual test scripts for both error handling and page size configuration.
Error handling improvements:
NotionApiExceptionclass that captures Notion-specific error codes and messages, and provides helper methods to easily check for common error types (e.g., unauthorized, not found, rate limited, etc.). All API errors now throw this exception. [1] [2] [3]Notionconnector to use theAlwaysThrowOnErrorstrait and to returnNotionApiExceptionfor failed requests, ensuring consistent error handling across the SDK. [1] [2]examples/error-handling-test.phpto demonstrate and verify the new error handling behavior and helper methods.Pagination and page size support:
Actionsresource to support automatic pagination forgetBlockChildrenandqueryDataSourcemethods. When requesting more than 100 items, the SDK now transparently fetches multiple pages and returns a combined result. [1] [2]BlockChildren,QueryDataSource) to accept and pass pagination parameters (page_size,start_cursor). [1] [2] [3]examples/page-size-test.phpto verify that thedefault_page_sizeconfiguration works as expected and that auto-pagination correctly retrieves large datasets.