Skip to content

Add cancellation token and default timeout (20s) to service requests#382

Merged
jramsay merged 6 commits intomainfrom
dev/jasonra/axios-optimizations
Jan 23, 2024
Merged

Add cancellation token and default timeout (20s) to service requests#382
jramsay merged 6 commits intomainfrom
dev/jasonra/axios-optimizations

Conversation

@jramsay
Copy link
Copy Markdown
Member

@jramsay jramsay commented Jan 22, 2024

Fixes #

Changes proposed:

  • add cancellation token to all tunnelManagementClient requests
  • add default connection/service response timeouts
  • add unit tests

Other Tasks:

  • If you updated the Go SDK did you update the PackageVersion in tunnels.go
  • [✅ ] If you updated the TS SDK did you update the dependencies in package.json for connections and management to require a dependency that is > the current published version(Found using npm view @microsoft/dev-tunnels-contracts). This will fix issues where yarn will pull the old version of packages and will cause mismatched dependencies. See example PR

@jramsay jramsay changed the title Add cancellation token and default timeout (20ms) to service requests Add cancellation token and default timeout (20s) to service requests Jan 22, 2024
Copy link
Copy Markdown

@osortega osortega left a comment

Choose a reason for hiding this comment

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

Would it be better to handle request retries completely within the SDK if there is already a way to track a request done through the SSH session?
This way it would be a transparent implementation on the user side instead of implementing the same thing for every API.

Comment thread ts/src/connections/tunnelRelayTunnelClient.ts Outdated
Comment thread ts/src/connections/tunnelRelayTunnelClient.ts Outdated
Comment thread ts/src/connections/package.json
Comment thread ts/src/connections/tunnelClient.ts Outdated
@jramsay jramsay merged commit 8cf0712 into main Jan 23, 2024
@jramsay jramsay deleted the dev/jasonra/axios-optimizations branch January 23, 2024 00:17
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.

3 participants