-
Notifications
You must be signed in to change notification settings - Fork 329
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(templates): add voice-queue template #149
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Jon, thanks for this, top idea for a function.
I am worried about the race condition within the enqueue function. Also, the test for that function doesn't test that the outbound call is placed. Could you take a look at those bits please?
3. Initiate a new project | ||
|
||
``` | ||
twilio serverless:init example --template=voice-queue && cd voice-queue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should name this project voice-queue
not example
(or change to cd
into example
).
if (!fromSid) { | ||
return callback({ | ||
status: 500, | ||
message: 'No CallSid found. Inbound voice webhook should be set to /enqueue' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning a string here should be sufficient to trigger a 500 error from the Function. Runtime, as far as I know, won't do anything special with this as an object.
const path = `https://${context.DOMAIN_NAME}`; | ||
// Make an outbound call to your phone number to connect the two legs. | ||
// Use the inbound CallSid as the unique identifier for the queue. | ||
client.calls.create({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a chance of a race condition here between creating the call and the function ending, as we don't wait for this asynchronous call to complete before calling back?
Also, should the function just fail if there's no From
?
So could this all be?
exports.handler = async function (context, event, callback) {
const { CallSid, From } = event;
if (!From) {
callback("Calls must have a `From`.");
} else {
const client = context.getTwilioClient();
const path = `https://${context.DOMAIN_NAME}`;
// Make an outbound call to your phone number to connect the two legs.
// Use the inbound CallSid as the unique identifier for the queue.
try {
await client.calls.create({
to: context.YOUR_PHONE_NUMBER,
from: context.TWILIO_PHONE_NUMBER,
url: `${path}/dial-queue?fromCallSid=${CallSid}&from=${From}`
});
// Enqueue the inbound caller.
// waitUrl points to an endpoint to <Play> the mp3 set in the env variables.
const response = new Twilio.twiml.VoiceResponse();
response.enqueue({
waitUrl: '/hold-music'
}, CallSid);
callback(null, response);
} catch (error) {
callback(error);
}
}
};
const context = {}; | ||
|
||
const event = { | ||
CallSid: 'CAX', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're missing a From
here, so the function never tests whether a call is placed. Check out other functions, like the airtable one, to see how to mock getTwilioClient
on the context
.
Description
Add template: voice-queue
Template Description: A simple call queuing system. Place the inbound caller into a queue with hold music while waiting for the intended recipient to answer.
Checklist
npm test
locally and it passed without errors.Related issues
None