Conversation
This commit introduces a `--timeout` option to the `md-to-notion-cli.ts` script.
The timeout is applied when calling the Notion API.
The default timeout is 10 seconds (10000ms).
Changes include:
- Added a `--timeout` option to the commander program definition in `src/md-to-notion-cli.ts`.
- Updated the `main` function to accept and parse the `timeout` option.
- Passed the parsed timeout value to the `timeoutMs` parameter of the Notion `Client` constructor.
- Added unit tests in `test/md-to-notion-cli.test.ts` to verify:
- Default timeout is applied.
- Custom timeout values are correctly passed.
- Timeout value is parsed as an integer.
- Manually tested the CLI with various timeout values to confirm expected behavior, including successful calls (with invalid token) and timeout errors with very short timeout values.
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a new --timeout option to the CLI tool for controlling the API call timeout behavior. Key changes include updating the CLI's options and main function in src/md-to-notion-cli.ts, comprehensive unit tests in test/md-to-notion-cli.test.ts to ensure proper behavior, and a manual test markdown file for additional verification.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/md-to-notion-cli.test.ts | Added tests to verify default, custom, and integer parsing behavior for the timeout. |
| temp_md_files_manual_test/test.md | Added a simple markdown file to facilitate manual testing of the timeout functionality. |
| src/md-to-notion-cli.ts | Updated the CLI option and main function to support the new --timeout parameter. |
|
|
||
| if (dir) { | ||
| const notion = new Client({ auth: options.token }) | ||
| const timeoutMs = parseInt(String(options.timeout), 10) |
There was a problem hiding this comment.
Consider adding validation to check if the result of parseInt is NaN and handle it appropriately, such as reverting to the default timeout or throwing a clear error.
| const timeoutMs = parseInt(String(options.timeout), 10) | |
| let timeoutMs = parseInt(String(options.timeout), 10) | |
| if (isNaN(timeoutMs)) { | |
| timeoutMs = 30000 // Default timeout value in milliseconds | |
| } |
| }) | ||
| }) | ||
|
|
||
| it("should parse timeout as an integer", async () => { |
There was a problem hiding this comment.
Consider documenting that the timeout value is truncated to an integer when a non-integer input is provided, to clarify the behavior for future maintainers and users.
1 participant
This commit introduces a
--timeoutoption to themd-to-notion-cli.tsscript.The timeout is applied when calling the Notion API. The default timeout is 10 seconds (10000ms).
Changes include:
--timeoutoption to the commander program definition insrc/md-to-notion-cli.ts.mainfunction to accept and parse thetimeoutoption.timeoutMsparameter of the NotionClientconstructor.test/md-to-notion-cli.test.tsto verify: