-
Notifications
You must be signed in to change notification settings - Fork 5
Sanitize phone number data before insertion and add optional voice_re… #15
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
Conversation
…ceive_mode field to schema to accomodate changing results from twilio
commit: |
…_sid, emergency_address_sid, identity_sid, and trunk_sid Fixes validation issues in production
WalkthroughAdds input validation for phone number insertion using a new convex-helpers dependency. Updates the phone_numbers schema to allow string values in previously null-only fields and introduces an optional voice_receive_mode field. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Mutation as insert Mutation
participant Validator as attemptToParse
participant Database as DB
Client->>Mutation: Insert phone_number
Mutation->>Validator: Parse phone_number with schema validator
alt Parsing succeeds
Validator->>Validator: { kind: "success", data: ParsedData }
Validator->>Mutation: Return success
Mutation->>Database: Insert validated data
Database->>Mutation: Success
Mutation->>Client: Phone number inserted
else Parsing fails
Validator->>Validator: { kind: "error", error: Err }
Validator->>Mutation: Return error
Mutation->>Mutation: Log error
Mutation->>Client: Throw "Invalid phone number data"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/component/phone_numbers.ts(2 hunks)src/component/schema.ts(3 hunks)src/component/utils.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/component/phone_numbers.ts (1)
src/component/utils.ts (1)
attemptToParse(42-57)
🔇 Additional comments (6)
src/component/phone_numbers.ts (1)
9-9: LGTM: Import added for validation helper.The import of
attemptToParsesupports the new input sanitization logic.src/component/utils.ts (2)
1-2: LGTM: Imports for validation utilities.The imports correctly bring in the necessary utilities from convex-helpers/validators for the new parsing functionality.
39-57: LGTM: Well-implemented validation helper.The
attemptToParsefunction is well-designed with:
- Proper generic typing that preserves validator types
- Discriminated union return type enabling type narrowing
- Clean error handling with try/catch
This provides a reusable pattern for safe parsing throughout the codebase.
src/component/schema.ts (2)
42-42: LGTM: Schema updated to accommodate Twilio's response changes.The fields are correctly updated from null-only to
v.union(v.string(), v.null()), allowing Twilio to return string values while maintaining backward compatibility with existing null values.Also applies to: 45-45, 54-54, 58-58, 73-73
81-81: LGTM: New optional field added.The
voice_receive_modefield is appropriately marked as optional, which won't break existing records in the database.package.json (1)
86-88: convex-helpers version 0.1.106 is valid and free from known vulnerabilities.The package version exists on npm and is the latest available release. GitHub's security database reports no advisories for this package. The dependency is safe to use.
| // It might have extra fields so we need to sanitize it before inserting into the table | ||
| const result = attemptToParse( | ||
| schema.tables.phone_numbers.validator, | ||
| args.phone_number, | ||
| ); | ||
|
|
||
| if (result.kind === "error") { | ||
| console.error("Failed to parse phone number data:", result.error); | ||
| throw new Error("Invalid phone number data"); | ||
| } | ||
|
|
||
| return await ctx.db.insert("phone_numbers", result.data); |
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.
🧩 Analysis chain
Script executed:
#!/bin/bash
# First, let's locate the attemptToParse function and understand the validation setup
fd -t f "phone_numbers" --type tsRepository: get-convex/twilio
<shell_result>
Repository: get-convex/twilio
Exit code: 2
stderr:
error: 'ts' isn't a valid value for '--type <filetype>'
[possible values: file, directory, symlink, executable, empty, socket, pipe]
For more information try '--help'
</shell_result>
Script executed:
#!/bin/bash
# Find phone_numbers files
fd "phone_numbers" -e ts
# Also look for attemptToParse definition
rg -n "attemptToParse" --type ts -A 5Repository: get-convex/twilio
<shell_result>
Repository: get-convex/twilio
Exit code: 0
stdout:
src/component/phone_numbers.ts
src/component/utils.ts:42:export function attemptToParse<T extends Validator<any, any, any>>(
src/component/utils.ts-43- validator: T,
src/component/utils.ts-44- value: unknown,
src/component/utils.ts-45-): { kind: "success"; data: Infer<T> } | { kind: "error"; error: unknown } {
src/component/utils.ts-46- try {
src/component/utils.ts-47- return {
--
src/component/phone_numbers.ts:9:import { twilioRequest, attemptToParse } from "./utils.js";
src/component/phone_numbers.ts-10-import schema from "./schema.js";
src/component/phone_numbers.ts-11-
src/component/phone_numbers.ts-12-export const create = action({
src/component/phone_numbers.ts-13- args: {
src/component/phone_numbers.ts-14- account_sid: v.string(),
--
src/component/phone_numbers.ts:44: const result = attemptToParse(
src/component/phone_numbers.ts-45- schema.tables.phone_numbers.validator,
src/component/phone_numbers.ts-46- args.phone_number,
src/component/phone_numbers.ts-47- );
src/component/phone_numbers.ts-48-
src/component/phone_numbers.ts-49- if (result.kind === "error") {
</shell_result>
Script executed:
#!/bin/bash
# Read the full attemptToParse function
cat -n src/component/utils.ts | sed -n '42,60p'
# Also check the schema file
fd "schema" -e tsRepository: get-convex/twilio
<shell_result>
Repository: get-convex/twilio
Exit code: 0
stdout:
42 export function attemptToParse<T extends Validator<any, any, any>>(
43 validator: T,
44 value: unknown,
45 ): { kind: "success"; data: Infer<T> } | { kind: "error"; error: unknown } {
46 try {
47 return {
48 kind: "success",
49 data: parse(validator, value),
50 };
51 } catch (error) {
52 return {
53 kind: "error",
54 error,
55 };
56 }
57 }
example/convex/schema.ts
src/component/schema.ts
</shell_result>
Script executed:
#!/bin/bash
# Check the schema definition
cat -n src/component/schema.ts | head -80
# Check imports in utils.ts to see where parse comes from
rg -n "import.*parse" src/component/utils.tsRepository: get-convex/twilio
<shell_result>
Repository: get-convex/twilio
Exit code: 0
stdout:
1 import { defineSchema, defineTable } from "convex/server";
2 import { v } from "convex/values";
3
4 export default defineSchema({
5 messages: defineTable({
6 account_sid: v.string(),
7 api_version: v.string(),
8 body: v.string(),
9 counterparty: v.optional(v.string()),
10 date_created: v.string(),
11 date_sent: v.union(v.string(), v.null()),
12 date_updated: v.union(v.string(), v.null()),
13 direction: v.string(),
14 error_code: v.union(v.number(), v.null()),
15 error_message: v.union(v.string(), v.null()),
16 from: v.string(),
17 messaging_service_sid: v.union(v.string(), v.null()),
18 num_media: v.string(),
19 num_segments: v.string(),
20 price: v.union(v.string(), v.null()),
21 price_unit: v.union(v.string(), v.null()),
22 sid: v.string(),
23 status: v.string(),
24 subresource_uris: v.union(
25 v.object({ media: v.string(), feedback: v.optional(v.string()) }),
26 v.null(),
27 ),
28 to: v.string(),
29 uri: v.string(),
30 rest: v.optional(v.any()),
31 })
32 .index("by_sid", ["account_sid", "sid"])
33 .index("by_account_sid", ["account_sid"])
34 .index("by_account_sid_and_direction", ["account_sid", "direction"])
35 .index("by_to", ["account_sid", "to"])
36 .index("by_from", ["account_sid", "from"])
37 .index("by_counterparty", ["account_sid", "counterparty"]),
38
39 phone_numbers: defineTable({
40 account_sid: v.string(),
41 address_requirements: v.string(),
42 address_sid: v.union(v.string(), v.null()),
43 api_version: v.string(),
44 beta: v.boolean(),
45 bundle_sid: v.union(v.string(), v.null()),
46 capabilities: v.object({
47 fax: v.boolean(),
48 mms: v.boolean(),
49 sms: v.boolean(),
50 voice: v.boolean(),
51 }),
52 date_created: v.string(),
53 date_updated: v.string(),
54 emergency_address_sid: v.union(v.string(), v.null()),
55 emergency_address_status: v.string(),
56 emergency_status: v.string(),
57 friendly_name: v.string(),
58 identity_sid: v.union(v.string(), v.null()),
59 origin: v.string(),
60 phone_number: v.string(),
61 sid: v.string(),
62 sms_application_sid: v.string(),
63 sms_fallback_method: v.string(),
64 sms_fallback_url: v.string(),
65 sms_method: v.string(),
66 sms_url: v.string(),
67 status: v.string(),
68 status_callback: v.string(),
69 status_callback_method: v.string(),
70 subresource_uris: v.object({
71 assigned_add_ons: v.string(),
72 }),
73 trunk_sid: v.union(v.string(), v.null()),
74 uri: v.string(),
75 voice_application_sid: v.string(),
76 voice_caller_id_lookup: v.boolean(),
77 voice_fallback_method: v.string(),
78 voice_fallback_url: v.string(),
79 voice_method: v.string(),
80 voice_url: v.string(),
1:import { parse } from "convex-helpers/validators";
</shell_result>
Script executed:
#!/bin/bash
# Search for any console.warn in the codebase related to validation or parsing
rg -n "console\.warn" --type ts
# Also check for any comments about extra fields or validation behavior
rg -n "extra.*field|sanitize|console\.warn" --type ts -B 2 -A 2Repository: get-convex/twilio
<shell_result>
Repository: get-convex/twilio
Exit code: 0
stdout:
src/component/phone_numbers.ts-41- returns: v.id("phone_numbers"),
src/component/phone_numbers.ts-42- handler: async (ctx, args) => {
src/component/phone_numbers.ts:43: // It might have extra fields so we need to sanitize it before inserting into the table
src/component/phone_numbers.ts-44- const result = attemptToParse(
src/component/phone_numbers.ts-45- schema.tables.phone_numbers.validator,
</shell_result>
convex-helpers parse function extra fields validation behavior
<web_search_result>
The parse(validator, value) helper both validates and strips unknown object fields — it returns a value that matches the validator exactly (removing extra keys). By contrast validate just checks (and can throw) without transforming/stripping. parse will still throw if required fields/types fail validation. [1][2]
Sources:
- convex-helpers validators (parse described as "validates and strips unknown fields"). [1]
- convex.dev docs / convex-helpers Zod integration (notes on parse vs validate and that parse/parse-like returns trimmed object and throws on invalid). [2]
</web_search_result>
The PR description claim about console.warn is inaccurate; the parse function silently strips extra fields without warnings.
The validation logic correctly uses parse() to sanitize phone number data before insertion. However, the PR description states "The validation will console.warn if there are extra fields," which is incorrect. The parse() function from convex-helpers automatically strips unknown fields silently without logging warnings or errors—it simply removes them and returns the validated object. Extra fields do not trigger console.warn; they are discarded without notification.
The error handling appropriately logs and throws only when validation fails (missing required fields or type mismatches). To align with actual behavior, update the PR description to clarify that extra fields are silently stripped rather than warned about.
🤖 Prompt for AI Agents
In src/component/phone_numbers.ts around lines 43 to 54, the PR description
incorrectly states that the validator will console.warn when extra fields are
present; in reality the parse() used by attemptToParse silently strips unknown
fields without logging. Update the PR description to state that extra fields are
silently removed by the validator (and that console.warn is not produced), and
keep the existing error handling unchanged since it already only logs/throws on
real validation failures.
|
Released in the latest version |
Twilio has changed their response schema, so we add a field to the table, and make our validation code more robust for the future.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.