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

[BUG] page takes 30 seconds to trigger status callback in backend #210

Closed
7 of 9 tasks
electrovir opened this issue Oct 16, 2023 · 14 comments
Closed
7 of 9 tasks

[BUG] page takes 30 seconds to trigger status callback in backend #210

electrovir opened this issue Oct 16, 2023 · 14 comments
Labels
bug Something isn't working

Comments

@electrovir
Copy link

electrovir commented Oct 16, 2023

  • I have verified that the issue occurs with the latest twilio.js release and is not marked as a known issue in the CHANGELOG.md.
  • I reviewed the Common Issues and open GitHub issues and verified that this report represents a potentially new issue.
  • I verified that the Quickstart application works in my environment.
  • I am not sharing any Personally Identifiable Information (PII)
    or sensitive account information (API keys, credentials, etc.) when reporting this issue.

Code to reproduce the issue

N/A

Expected behavior

(This is the behavior from version 1 of the Twilio client)

  1. frontend Twilio device receives a call
  2. frontend user has not accepted the call yet
  3. frontend user refreshes their page
  4. "Status Callback URL" on our backend is immediately hit from Twilio
  5. We know immediately that the frontend user "missed" the call, in effect, because they refreshed their page

Actual behavior

Step 4 in the above expected behavior now takes 30 seconds (almost exactly) every time those steps are followed.

Is there a config we're missing somewhere with our Twilio upgrade to fire status callbacks more quickly?

Software versions

  • Browser(s): most recent Chrome and Safari versions
  • Operating System: macOS Sonoma
  • twilio.js:
  • @twilio/voice-sdk: 2.8.0
  • Third-party libraries (e.g., Angular, React, etc.): none
@electrovir electrovir added the bug Something isn't working label Oct 16, 2023
@charliesantos
Copy link
Collaborator

Thanks for submitting @electrovir . Just to confirm, this is happening on 2.x only and not on 1.x correct?

@electrovir
Copy link
Author

I browsed Device Options and found nothing that looked relevant.

My only next idea is to add our own beforeunload event listener and reject the call inside of that. Why wasn't this necessary on the previous twilio client version though?

@electrovir
Copy link
Author

Thanks for submitting @electrovir . Just to confirm, this is happening on 2.x only and not on 1.x correct?

That is correct @charliesantos. Swapping back to version 1.x fixes this.

@charliesantos
Copy link
Collaborator

@electrovir another quick question, are you passing any DeviceOptions.closeProtection parameter to the device on either 1.x or 2.x?

@electrovir
Copy link
Author

@electrovir another quick question, are you passing any DeviceOptions.closeProtection parameter to the device on either 1.x or 2.x?

closeProtection has not been set in our code in either version.

I've tried setting it to true on 2.x and that did not help.

@electrovir
Copy link
Author

electrovir commented Oct 16, 2023

Adding the following code in our Device.EventName.Incoming event handler worked:

window.onbeforeunload = event => {
    twilioCall.reject();
    return false;
};

However, that means we can't add an actual "are you sure you want to navigate away" pop-up from onbeforeunload (by returning something truthy) because we won't be able to reliably detect if the user clicked "stay on page" or "yes I want to leave the page". So we'd have to let them navigate away and reject the call. (Which, to be fair, is what 1.x did.)

@charliesantos
Copy link
Collaborator

Hi @electrovir we are able to reproduce it and for some reason, the incoming calls are not properly getting tracked inside our unload handler. I'll submit an internal ticket to address this. As a workaround, I think you need to also subscribe to window.onunload. Basically:

  • beforeunload event should be used to pop up a dialog "are you sure you want to navigate away"
  • unload event is where you should do call.reject() or also invoke call.disconnect() on any active calls.

@electrovir
Copy link
Author

also subscribe to window.onunload

True, though I've always avoided relying on that event because:

Warning: Developers should avoid using this event. See "Usage notes" below.

(from the MDN docs)

Though, if that's what 1.x used (an unload listener) then I guess it was working well enough for us to use it again now 😁

@charliesantos
Copy link
Collaborator

Correct, it's what both 1.x and 2.x uses. Watch out for a future release that contains the fix. And don't forget to remove your workaround after upgrading 😄

@electrovir
Copy link
Author

Will do, thank you!

@electrovir
Copy link
Author

electrovir commented Oct 17, 2023

Some interesting results from my workaround experiments:

  • Calling Call.reject() inside of an unload event listener (either with window.addEventLister('unload' or window.onunload) does not work. I presume that what's happening is that the browser page terminates before the results of calling reject() have time to propagate to Twilio.
  • Calling Call.reject() inside of a beforeunload event listener (either with window.addEventListener('beforeunload' or window.onbeforeunload) does work, even if it returns an empty string (which doesn't trigger a pop-up).

tl;dr using the unload event is indeed unreliable, at least in my simple workarounds.

(Tested on both Safari and Chrome.)

@charliesantos
Copy link
Collaborator

Let's track this issue here #118

@charliesantos
Copy link
Collaborator

Hey everyone, are you all passing any value to maxCallSignalingTimeoutMs

@charliesantos
Copy link
Collaborator

fixed in 2.10.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants