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

Handle voice playback failure #731

Closed
wants to merge 3 commits into from
Closed

Handle voice playback failure #731

wants to merge 3 commits into from

Conversation

iAmmar7
Copy link
Collaborator

@iAmmar7 iAmmar7 commented Jan 27, 2023

On playback failure

  • Emit playback.failed event to the end-user,
  • Resolve the playback promise

This PR also includes an e2e test case for the playback function.

@iAmmar7 iAmmar7 requested a review from edolix January 27, 2023 11:56
@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2023

🦋 Changeset detected

Latest commit: 5d69bb8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@signalwire/realtime-api Minor
@signalwire/core Minor
@signalwire/js Patch
@signalwire/react-native Patch
@signalwire/web-api Patch
@signalwire/webrtc Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -104,6 +104,10 @@ export class CallPlaybackAPI
}

ended() {
if (['finished', 'error'].includes(this.state)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Can you create a const at the top with the states to check? Like ENDED_STATES = [...]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done ✅

resolve(true)
})
})
await waitForPlaybackEnded
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed already: lets add a comment explaining the logic in here since the playback ends because the inbound call hangup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done ✅ Let me know if I can improve it further

.changeset/seven-beers-occur.md Show resolved Hide resolved
@edolix
Copy link
Contributor

edolix commented Jan 31, 2023

See #732

@edolix edolix closed this Jan 31, 2023
@edolix edolix deleted the aa/playback-failure branch June 23, 2023 08:01
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