fix(mp): handle non-string messageId to prevent slice error

[maheshkutty]

What are the changes introduced in this PR?

Fixes runtime error when messageId is not a string type (number, boolean, object). The code was unconditionally calling .slice(-36) on $insert_id without type checking, causing "mappedProperties.$insert_id.slice is not a function" errors in production.

Changes:

• Add type check to only apply .slice(-36) when $insert_id is a string
• Non-string messageId values are now passed through unchanged
• Prevents 500 errors and customer data loss

What is the related Linear task?

Resolves INT-4316

Please explain the objectives of your changes below

Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here

Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?

N/A

Any new dependencies introduced with this change?

N/A

Any new generic utility introduced or modified. Please explain the changes.

N/A

Any technical or performance related pointers to consider with the change?

N/A

@coderabbitai review

---

Developer checklist

• [ ] My code follows the style guidelines of this project

• [ ] No breaking changes are being introduced.

• [ ] All related docs linked with the PR?

• [ ] All changes manually tested?

• [ ] Any documentation changes needed with this change?

• [ ] Is the PR limited to 10 file changes?

• [ ] Is the PR limited to one linear task?

• [ ] Are relevant unit and component test-cases added in new readability format?

Reviewer checklist

• [ ] Is the type of change in the PR title appropriate as per the changes?

• [ ] Verified that there are no credentials or confidential data exposed with the changes.

[coderabbitai[bot]]

[!NOTE]

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)

Validation error: Unrecognized key(s) in object: 'auto_resolve_threads'

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Summary by CodeRabbit

• Bug Fixes
  • Enhanced data validation to catch and report invalid message identifiers, improving error handling and preventing downstream processing issues.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

Adds a runtime guard in the Mixpanel transformer: if message.messageId exists and is not a string, throw InstrumentationError('messageId should be of type string'). Retains existing behavior of slicing mappedProperties.$insert_id to the last 36 characters when present and updates related comments.

Changes

Cohort / File(s)	Summary
Mixpanel transform runtime checks src/v0/destinations/mp/transform.js	Added a runtime type guard validating message.messageId is a string and throwing InstrumentationError('messageId should be of type string') when it exists but is not a string. Kept existing behavior of slicing mappedProperties.$insert_id to .slice(-36) when present; comments updated to clarify messageId expectations and timestamp note.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

• Verify the InstrumentationError message, type, and import are correct and consistent with project conventions.
• Confirm callers/consumers handle this error path appropriately (or that errors should surface).
• Check tests or add a unit test covering non-string messageId inputs and the preserved $insert_id slicing behavior.
• Ensure comments accurately reflect intent; update docs if messageId typing is part of public contract.

Possibly related PRs

• rudderlabs/rudder-transformer#4559 — modifies runtime handling/propagation of messageId in user transforms; related by overlapping messageId validation/propagation logic.

Suggested reviewers

• vinayteki95
• koladilip
• sivashanmukh

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main change: adding a runtime guard to handle non-string messageId values and prevent slice errors.
Description check	✅ Passed	The description follows the template structure and provides clear details about the problem, solution, and linear task reference, with all required sections addressed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment
• [ ] Commit unit tests in branch fix.mp_insert_id_slice

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb0fc440e4e7f742cbcb39af07a265f030874aca and 9129418a28868c0c86db6933d1215400caf5bda6.

⛔ Files ignored due to path filters (1)

• test/integrations/destinations/mp/processor/data.ts is excluded by !**/test/**

📒 Files selected for processing (1)

• src/v0/destinations/mp/transform.js (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.js

⚙️ CodeRabbit configuration file

Focus on ESLint errors (max 3) and warnings (max 5).

Files:

• src/v0/destinations/mp/transform.js

🧠 Learnings (1)

📚 Learning: 2025-05-29T13:29:39.436Z

Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4359
File: src/v0/util/index.js:2272-2272
Timestamp: 2025-05-29T13:29:39.436Z
Learning: The `combineBatchRequestsWithSameJobIds` function in `src/v0/util/index.js` is used by both Mixpanel and Google AdWords Offline Conversions destinations in production. The function assumes metadata is always an array in multiple operations (forEach, filter, sort, map) and needs defensive programming to handle non-array metadata cases to prevent runtime errors.

Applied to files:

• src/v0/destinations/mp/transform.js

🧬 Code graph analysis (1)

src/v0/destinations/mp/transform.js (1)

src/v0/util/index.js (3)

• message (1157-1157)
• message (1191-1191)
• constructPayload (1068-1142)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

• GitHub Check: Build Transformer Docker Image - PR / Build Transformer Docker Image AMD64
• GitHub Check: Build Transformer Docker Image - PR / Build Transformer Docker Image ARM64
• GitHub Check: Build User Transformer Docker Image - PR / Build Transformer Docker Image ARM64
• GitHub Check: Build User Transformer Docker Image - PR / Build Transformer Docker Image AMD64
• GitHub Check: Code Coverage
• GitHub Check: check-health
• GitHub Check: Check for formatting & lint errors
• GitHub Check: UT Tests
• GitHub Check: test_and_publish
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (go)

🔇 Additional comments (3)

src/v0/destinations/mp/transform.js (3)

175-180: Improved documentation and safe slice operation.

The updated comments effectively explain that SDKs send timestamps in messageId, and the documentation reference at line 177 adds valuable context. The slice operation at line 179 should now be safe, as the validation at lines 169-172 ensures messageId is a string when present.

---

169-172: PR summary inconsistency with implementation.

The PR summary states "Non-string messageId values are passed through unchanged," but the code actually throws an InstrumentationError when messageId exists and is not a string. The implementation correctly reflects the team's decision from the review discussion (throw an error for non-string values), but the PR description should be updated to accurately describe this behavior.

---

169-172: Validation logic appears sound, but mapping configuration could not be verified to confirm exclusive messageId→$insert_id derivation.

The validation at lines 169-172 correctly throws an error when messageId exists and is not a string, which protects the slice operation at line 179. However, verification of the mapping relationship could not be completed:

• The constructPayload function requires mPEventPropertiesConfigJson to map message.messageId to mappedProperties.$insert_id
• The expected mapping configuration file (src/v0/destinations/mp/data/MPEventPropertiesConfig.json) could not be located in the repository
• Without access to this configuration, we cannot definitively verify that $insert_id is exclusively derived from message.messageId

The validation approach is logically correct—it checks message.messageId before constructPayload processes it. However, if the mapping configuration maps $insert_id from sources other than messageId, this validation alone would be insufficient to prevent slice errors.

Recommendation: Verify that the mapping configuration only derives $insert_id from message.messageId, or add additional defensive checks if other sources can populate $insert_id.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[devops-github-rudderstack]

Allure Test reports for this run are available at:

• Allure Report: View Report

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 91.99%. Comparing base (b50d99d) to head (9129418). :warning: Report is 3 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4824   +/-   ##
========================================
  Coverage    91.99%   91.99%           
========================================
  Files          655      655           
  Lines        34962    34964    +2     
  Branches      8230     8230           
========================================
+ Hits         32163    32165    +2     
  Misses        2554     2554           
  Partials       245      245           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[devops-github-rudderstack]

Allure Test reports for this run are available at:

• Allure Report: View Report

[devops-github-rudderstack]

Allure Test reports for this run are available at:

• Allure Report: View Report

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud