Skip to content

Improve sample Twilio connector code - #137

Open
PasinduGunarathne wants to merge 1 commit into
wso2:mainfrom
PasinduGunarathne:pasindug-patch-3
Open

Improve sample Twilio connector code#137
PasinduGunarathne wants to merge 1 commit into
wso2:mainfrom
PasinduGunarathne:pasindug-patch-3

Conversation

@PasinduGunarathne

Copy link
Copy Markdown

Purpose

  • Improve the sample code of the Twilio Ballerina connector publishing flow and update the integration samples

Goals

Describe the solutions that this feature/fix will introduce to resolve the problems described above

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI (email documentation@wso2.com to review all UI text). Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

User stories

Summary of user stories addressed by this change>

Release note

Brief description of the new feature or bug fix as it will appear in the release notes

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter “N/A” plus brief explanation of why there’s no doc impact

Training

Link to the PR for changes to the training content in https://github.com/wso2/WSO2-Training, if applicable

Certification

Type “Sent” when you have provided new/updated certification questions, plus four answers for each question (correct answer highlighted in bold), based on this change. Certification questions/answers should be sent to certification@wso2.com and NOT pasted in this PR. If there is no impact on certification exams, type “N/A” and explain why.

Marketing

Link to drafts of marketing content that will describe and promote this feature, including product page changes, technical articles, blog posts, videos, etc., if applicable

Automation tests

  • Unit tests

    Code coverage information

  • Integration tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Migrations (if applicable)

Describe migration steps and platforms on which migration has been tested

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Learning

Describe the research phase and any blog posts, patterns, libraries, or add-ons you used to solve the problem.

@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Changes Summary

This pull request enhances the Twilio connector sample code by expanding the automation workflow and improving the trigger sample implementation.

Configuration Enhancements

The Twilio connector configuration now includes additional parameters for controlling call behavior: fromNumber, toNumber, statusCallbackUrl, twimlUrl, and a default callMessage string. These parameters enable more flexible configuration of the connector's call and messaging capabilities.

New TwiML HTTP Service

A new file introduces a dedicated TwiML HTTP service that responds to voice requests with dynamic XML responses. The service is configured to run on a separate port (8091) and incorporates the configurable message text into the TwiML response payload. This service supports the callback mechanism for Twilio voice interactions.

Improved Automation Sample

The automation sample workflow now initiates a Twilio call before sending a message, using the newly configurable parameters. Both operations extract and log their respective identifiers (call SID and message SID), providing better visibility into the connector's activity. The previous hardcoded inline message creation was replaced with parameterized calls.

Enhanced Trigger Sample

The trigger sample has been refactored to simplify SMS status handling and add comprehensive call status logging. Key improvements include:

  • Simplified listener instantiation using positional arguments
  • Streamlined SMS status handlers with structured field logging instead of full JSON dumps
  • New call status service that monitors multiple call lifecycle states (queued, ringing, in progress, completed, busy, failed, no answer, canceled)
  • Consistent error handling pattern across all remote functions
  • Better observability through specific field extraction with null-coalescing operators

These changes provide a more complete example of Twilio integration capabilities, including both outbound messaging/calling and inbound webhook handling.

Walkthrough

The pull request enhances two Twilio integration samples. The connector sample extends its automation to initiate voice calls before sending messages, introducing configurable parameters for call routing and callbacks. A new TwiML HTTP endpoint serves dynamic call scripts. The trigger sample refactors error handling for consistency, standardizes return type signatures across status callbacks, and adds comprehensive call status lifecycle tracking alongside existing SMS status handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes only the template placeholder structure with minimal content; the 'Purpose' section is partially filled but all other required sections lack substantive information. Complete missing sections including Goals, Approach, Release note, Documentation, Samples, Test environment, and Security checks with specific details about the changes made.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Improve sample Twilio connector code' accurately reflects the main objective of the changeset—enhancing the Twilio connector sample implementation across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@integrator-default-profile/connectors/twilio_connector_sample/main.bal`:
- Around line 12-20: Update the Content-Type header in the TwiML responder:
inside the service /twiml's resource function post voice() where twimlResponse
is created and setHeader("Content-Type", "application/xml") is called, change
the header value to "text/xml"; optionally, when building the payload string for
twimlResponse.setPayload(...), add language and voice attributes to the <Say>
verb (e.g., language="en-US" voice="man") if desired.

In `@integrator-default-profile/connectors/twilio_trigger_sample/main.bal`:
- Around line 7-42: The extra Twilio status callbacks onBusy, onFailed,
onNoAnswer, and onCanceled are unused for voice StatusCallbackEvent and should
be removed or clearly documented if they target a different Twilio service;
update the service by deleting these remote functions (onBusy, onFailed,
onNoAnswer, onCanceled) if you only handle voice events, or add a comment
explaining their intended purpose and the alternative Twilio event types if
they’re required for messaging/other webhooks, keeping only onQueued, onRinging,
onInProgress, and onCompleted for voice.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a3bca0ea-8d4e-4892-89fc-e3af5e153b5f

📥 Commits

Reviewing files that changed from the base of the PR and between b847cf3 and 0961fcc.

📒 Files selected for processing (4)
  • integrator-default-profile/connectors/twilio_connector_sample/automation.bal
  • integrator-default-profile/connectors/twilio_connector_sample/config.bal
  • integrator-default-profile/connectors/twilio_connector_sample/main.bal
  • integrator-default-profile/connectors/twilio_trigger_sample/main.bal

Comment on lines +12 to +20
service /twiml on new http:Listener(twimlPort) {
resource function post voice() returns http:Response {
http:Response twimlResponse = new;
twimlResponse.setHeader("Content-Type", "application/xml");
twimlResponse.setPayload(string `<?xml version="1.0" encoding="UTF-8"?><Response><Say>${callMessage}</Say></Response>`);
log:printInfo(`TwiML ${callMessage} response sent`);
return twimlResponse;
}
} No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What is the correct TwiML XML format and Content-Type header for Twilio voice call responses with the Say verb?

💡 Result:

Correct TwiML XML format (for a Voice response that uses the verb): Hello! This must be valid TwiML: the root element is and all verbs (like ) must be nested inside it.[1] Correct Content-Type header for the HTTP response containing that TwiML: Content-Type: text/xml Twilio’s docs state that when your application responds with XML, Twilio interprets it as TwiML when the response Content-Type is one of text/xml, application/xml, or text/html; text/xml is the most commonly used value.[1] For status-callback endpoints specifically, Twilio says to respond with 200 OK and Content-Type: text/xml (with an empty body, or use 204 No Content).[1] Note: If you’re returning TwiML from your webhook (not just a status callback), you still generally want XML content types; per Twilio, text/xml/application/xml/text/html are treated as the TwiML XML Instruction Set.[1]

Citations:


Use text/xml for the Content-Type header instead of application/xml.

Twilio's documentation indicates that while application/xml is accepted, text/xml is the most commonly used and recommended Content-Type for TwiML responses. The XML structure and <Say> verb formatting are correct; however, the header should be changed from "application/xml" to "text/xml" to align with Twilio best practices.

Optionally, consider adding language and voice attributes to the <Say> verb (e.g., language="en-US" and voice="man"), though these are not required.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@integrator-default-profile/connectors/twilio_connector_sample/main.bal`
around lines 12 - 20, Update the Content-Type header in the TwiML responder:
inside the service /twiml's resource function post voice() where twimlResponse
is created and setHeader("Content-Type", "application/xml") is called, change
the header value to "text/xml"; optionally, when building the payload string for
twimlResponse.setPayload(...), add language and voice attributes to the <Say>
verb (e.g., language="en-US" voice="man") if desired.

Comment on lines +7 to 42
service twilio:CallStatusService on twilioListener {

remote function onQueued(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call queued", callSid = event.CallSid ?: "");
}

remote function onRinging(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call ringing", callSid = event.CallSid ?: "");
}

remote function onInProgress(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call in progress", callSid = event.CallSid ?: "");
}

remote function onCompleted(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call completed",
callSid = event.CallSid ?: "",
duration = event.CallDuration ?: "");
}

remote function onBusy(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call busy", callSid = event.CallSid ?: "");
}

remote function onFailed(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call failed", callSid = event.CallSid ?: "");
}

remote function onNoAnswer(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call no answer", callSid = event.CallSid ?: "");
}

remote function onCanceled(twilio:CallStatusEventWrapper event) returns error? {
log:printInfo("Call canceled", callSid = event.CallSid ?: "");
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check Twilio trigger connector documentation for event name mappings
rg -n "StatusCallbackEvent|onQueued|onInProgress|initiated|answered" --type=bal -C 3

Repository: wso2/integration-samples

Length of output: 96


🌐 Web query:

What are the valid StatusCallbackEvent values for Twilio voice calls and how do they map to Ballerina Twilio trigger service callback methods?

💡 Result:

Valid Twilio Voice StatusCallbackEvent values (Programmable Voice / REST or TwiML) are: initiated, ringing, answered, completed. Twilio’s voice webhooks docs explicitly list these events as the supported StatusCallbackEvent values when subscribing to additional status events beyond the default completed callback.[1] How they map to Ballerina Twilio trigger service callback methods (ballerinax/trigger.twilio Call status events): - initiated -> remote function onQueued(...) [2] - ringing -> remote function onRinging(...) [2] - answered -> remote function onInProgress(...) [2] - completed -> remote function onCompleted(...) [2] Note: The Ballerina trigger uses CallStatusEventWrapper callback method names (onQueued/onRinging/onInProgress/onCompleted) rather than the exact Twilio StatusCallbackEvent strings; the mapping above reflects which wrapper callback names the trigger exposes for those same call lifecycle phases.[2][1]

Citations:


Remove unused Twilio status callbacks or clarify their purpose.

The service defines callbacks for onBusy, onFailed, onNoAnswer, and onCanceled, but Twilio voice webhooks only support four StatusCallbackEvent values: initiated, ringing, answered, and completed. These four additional callbacks will never be invoked. Either remove them or document if they are intended for a different Twilio service type (e.g., messaging). The currently configured callbacks (onQueued, onRinging, onInProgress, onCompleted) correctly map to their respective Twilio events.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@integrator-default-profile/connectors/twilio_trigger_sample/main.bal` around
lines 7 - 42, The extra Twilio status callbacks onBusy, onFailed, onNoAnswer,
and onCanceled are unused for voice StatusCallbackEvent and should be removed or
clearly documented if they target a different Twilio service; update the service
by deleting these remote functions (onBusy, onFailed, onNoAnswer, onCanceled) if
you only handle voice events, or add a comment explaining their intended purpose
and the alternative Twilio event types if they’re required for messaging/other
webhooks, keeping only onQueued, onRinging, onInProgress, and onCompleted for
voice.

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.

1 participant