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

Add the ability to switch transport API #435

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Add the ability to switch transport API #435

merged 2 commits into from
Feb 5, 2025

Conversation

parfeon
Copy link
Contributor

@parfeon parfeon commented Feb 4, 2025

feat(transport): add the ability to switch transport API

For the browser version of PubNub SDK, add the ability to switch between fetch and xhr APIs (transport configuration option).

fix(event-engine): handshake/receive requests timeout

Fix issue because of which, in Event Engine mode, wrong timeout values have been set for requests which create long-poll request.

refactor(request): make sure request cancels on timeout

Refactor timeout implementation for fetch transport to properly cancel request when the timeout timer will fire.

For the browser version of PubNub SDK, add the ability to switch between `fetch` and `xhr` APIs
(`transport` configuration option).

fix(event-engine): handshake/receive requests timeout

Fix issue because of which, in Event Engine mode, wrong timeout values have been set for requests
which create long-poll request.

refactor(request): make sure request cancels on timeout

Refactor `timeout` implementation for `fetch` transport to properly cancel request when the
timeout timer will fire.
@parfeon parfeon added status: done This issue is considered resolved. priority: high This PR should be reviewed ASAP. type: fix This PR contains fixes to existing features. labels Feb 4, 2025
@parfeon parfeon self-assigned this Feb 4, 2025
@@ -118,6 +132,7 @@ export const setDefaults = (configuration: PubNubConfiguration): PubNubConfigura
listenToBrowserNetworkEvents: configuration.listenToBrowserNetworkEvents ?? LISTEN_TO_BROWSER_NETWORK_EVENTS,
subscriptionWorkerUrl: configuration.subscriptionWorkerUrl,
subscriptionWorkerLogVerbosity: configuration.subscriptionWorkerLogVerbosity ?? SUBSCRIPTION_WORKER_LOG_VERBOSITY,
transport: configuration.transport ?? TRANSPORT,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

*
* @default `fetch`
*/
transport?: 'fetch' | 'xhr';
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

*/
private async sendXHRRequest(req: WebTransportRequest, controller: WebCancellationController): Promise<Response> {
return new Promise<Response>((resolve, reject) => {
const xhr = new XMLHttpRequest();
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we go again!!!! :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a backup for really messed up case with APM tools and some weird browser restrictions which maybe enforced... fetch is shiny and better :)

@parfeon
Copy link
Contributor Author

parfeon commented Feb 5, 2025

@pubnub-release-bot release

@parfeon parfeon merged commit 07b3418 into master Feb 5, 2025
6 of 7 checks passed
@parfeon parfeon deleted the CLEN-2521 branch February 5, 2025 10:28
@pubnub-release-bot
Copy link
Contributor

🚀 Release successfully completed 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high This PR should be reviewed ASAP. status: done This issue is considered resolved. type: fix This PR contains fixes to existing features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants