Skip to content
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

feat: specifying socket timeout #410

Merged
merged 5 commits into from
Jul 26, 2023
Merged

Conversation

hung-cybo
Copy link
Contributor

@hung-cybo hung-cybo commented Jul 25, 2023

Why

When the user cannot execute cli-kintone caused of the user's network error, cli-kintone should finish the process and show the error without waiting time.

What

  • Specifying socket timeout. The default value is 60 seconds

How to test

Checklist

  • Read CONTRIBUTING.md
  • Updated documentation if it is required.
  • Added tests if it is required.
  • Passed pnpm lint and pnpm test on the root directory.

@hung-cybo hung-cybo requested review from a team and tasshi-me and removed request for a team July 25, 2023 07:53
src/utils/error.ts Outdated Show resolved Hide resolved
@@ -49,6 +54,22 @@ export abstract class CliKintoneError extends Error {
}
}

private _networkError(error: unknown): error is NetworkError {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I have one more proposal.
The type guard function is better to be named as isXXX().

Suggested change
private _networkError(error: unknown): error is NetworkError {
private _isNetworkError(error: unknown): error is NetworkError {

Copy link
Member

@tasshi-me tasshi-me left a comment

Choose a reason for hiding this comment

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

LGTM!

@tasshi-me tasshi-me merged commit b2cdb29 into main Jul 26, 2023
@tasshi-me tasshi-me deleted the feat/specify_socket_timeout branch July 26, 2023 06:08
This was referenced Jan 17, 2024
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