-
-
Notifications
You must be signed in to change notification settings - Fork 77
fix(apple): Change fastlane sentry_cli to sentry_debug_files_upload #1125
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
base: master
Are you sure you want to change the base?
Conversation
| ): string { | ||
| const laneContent = content.slice(lane.index, lane.index + lane.length); | ||
| const sentryCLIMatch = /sentry_cli\s*\([^)]+\)/gim.exec(laneContent); | ||
| const sentryCLIMatch = /(\n\s+)sentry_debug_files_upload\s*\([^)]+\)/gim.exec( |
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.
Bug: The addSentryToLane function fails to replace sentry_cli with sentry_debug_files_upload, leading to duplicate Sentry calls.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The addSentryToLane function's regex sentryCLIMatch = /(\n\s+)sentry_debug_files_upload\s*\([^)]+\)/gim.exec(laneContent); on line 82 only matches sentry_debug_files_upload. If a user's Fastfile contains the old sentry_cli function, this regex will not match. This causes the code to insert a new sentry_debug_files_upload block without removing the existing sentry_cli block, resulting in a misconfigured Fastfile with both old and new Sentry calls.
💡 Suggested Fix
Update the regex in addSentryToLane to match both sentry_cli and sentry_debug_files_upload, or add logic to explicitly remove sentry_cli before inserting the new sentry_debug_files_upload block.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/apple/fastlane.ts#L82
Potential issue: The `addSentryToLane` function's regex `sentryCLIMatch =
/(\n\s+)sentry_debug_files_upload\s*\([^)]+\)/gim.exec(laneContent);` on line 82 only
matches `sentry_debug_files_upload`. If a user's Fastfile contains the old `sentry_cli`
function, this regex will not match. This causes the code to insert a new
`sentry_debug_files_upload` block without removing the existing `sentry_cli` block,
resulting in a misconfigured Fastfile with both old and new Sentry calls.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 4205707
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.
I think this is better than replacing sentry_cli as it could still be that users correctly use an older fastlane plugin version where it is set up. Happy for feedback on this one.
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.
What bout matching on both sentry_cli and sentry_debug_files_upload?
| ): string { | ||
| const laneContent = content.slice(lane.index, lane.index + lane.length); | ||
| const sentryCLIMatch = /sentry_cli\s*\([^)]+\)/gim.exec(laneContent); | ||
| const sentryCLIMatch = /(\n\s+)sentry_debug_files_upload\s*\([^)]+\)/gim.exec( |
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.
What bout matching on both sentry_cli and sentry_debug_files_upload?
| ): string { | ||
| const laneContent = content.slice(lane.index, lane.index + lane.length); | ||
| const sentryCLIMatch = /sentry_cli\s*\([^)]+\)/gim.exec(laneContent); | ||
| const sentryCLIMatch = /(\n\s+)sentry_debug_files_upload\s*\([^)]+\)/gim.exec( |
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.
l: The const sentryCLIMatch now matches the sentry_debug_files_upload. I guess the variable name should reflect that.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1125 +/- ##
==========================================
+ Coverage 35.86% 35.87% +0.01%
==========================================
Files 144 144
Lines 17695 17698 +3
Branches 1416 1416
==========================================
+ Hits 6346 6349 +3
+ Misses 11331 11329 -2
- Partials 18 20 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The lane name changed from
sentry_clitosentry_debug_files_uploadcausing issues such as #1054.This PR uses the correct lane as implemented here:
https://github.com/getsentry/sentry-fastlane-plugin/blob/30ff38f493d567da9aa23954b15d4024609e4baf/lib/fastlane/plugin/sentry/actions/sentry_debug_files_upload.rb#L1-L5
Closes #1054