Skip to content

Conversation

limbonaut
Copy link
Collaborator

@limbonaut limbonaut commented Aug 8, 2025

📜 Description

This PR introduces new fields to the SentryFrame class: contextLine, preContext, postContext, and vars.

💡 Motivation and Context

These changes are needed for the Godot SDK to be able to capture script context and variables on iOS and macOS. We already have a similar setup with Sentry Native and Sentry Android backends.

Before this PR:

Screenshot 2025-08-08 at 14 25 46

After this PR:

Screenshot 2025-08-07 at 16 27 01

💚 How did you test it?

I’ve added unit tests.
Additionally, I used the WIP branch in Godot SDK to verify this functionality with a local Cocoa SDK build: getsentry/sentry-godot#306. The screenshot above was made using the linked branch.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 39664d5

@limbonaut limbonaut changed the title Feat/add frame source context feat: Add source context and vars fields to SentryFrame Aug 8, 2025
Copy link

codecov bot commented Aug 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.687%. Comparing base (8d944ac) to head (39664d5).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #5853   +/-   ##
=========================================
  Coverage   86.686%   86.687%           
=========================================
  Files          423       423           
  Lines        36527     36537   +10     
  Branches     17281     17288    +7     
=========================================
+ Hits         31664     31673    +9     
- Misses        4815      4819    +4     
+ Partials        48        45    -3     
Files with missing lines Coverage Δ
Sources/Sentry/SentryFrame.m 100.000% <100.000%> (ø)
...es/Swift/Protocol/Codable/SentryFrameCodable.swift 100.000% <100.000%> (ø)

... and 9 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d944ac...39664d5. Read the comment docs.

@limbonaut limbonaut marked this pull request as ready for review August 8, 2025 14:02
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@philprime philprime left a comment

Choose a reason for hiding this comment

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

LGTM, please fix the missing assertion.

I'll approve it, but I would also appreciate @philipphofmann take a look to confirm the changes, as they are public facing.

@limbonaut
Copy link
Collaborator Author

Missing decoding test check addressed in 158d1e0

Copy link
Contributor

github-actions bot commented Aug 12, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1211.53 ms 1235.98 ms 24.45 ms
Size 23.75 KiB 920.61 KiB 896.87 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
409a607 1229.57 ms 1251.45 ms 21.88 ms
f5d202b 1237.90 ms 1259.49 ms 21.59 ms
4d264fa 1223.48 ms 1246.91 ms 23.44 ms
102cf89 1218.31 ms 1239.78 ms 21.47 ms
7908e84 1224.33 ms 1246.39 ms 22.06 ms
1b9991e 1233.45 ms 1256.61 ms 23.17 ms
cd9727b 1236.04 ms 1254.41 ms 18.37 ms
15c8cd5 1220.24 ms 1246.14 ms 25.90 ms
db9572a 1223.13 ms 1241.60 ms 18.47 ms
5cfc768 1220.74 ms 1245.06 ms 24.32 ms

App size

Revision Plain With Sentry Diff
409a607 23.74 KiB 874.08 KiB 850.33 KiB
f5d202b 23.75 KiB 904.53 KiB 880.78 KiB
4d264fa 23.74 KiB 874.07 KiB 850.33 KiB
102cf89 23.74 KiB 891.02 KiB 867.27 KiB
7908e84 23.74 KiB 872.75 KiB 849.00 KiB
1b9991e 23.75 KiB 908.01 KiB 884.26 KiB
cd9727b 23.75 KiB 879.25 KiB 855.51 KiB
15c8cd5 23.75 KiB 908.01 KiB 884.26 KiB
db9572a 23.75 KiB 858.64 KiB 834.89 KiB
5cfc768 23.75 KiB 850.73 KiB 826.98 KiB

Previous results on branch: feat/add-frame-source-context

Startup times

Revision Plain With Sentry Diff
b915d28 1218.52 ms 1243.54 ms 25.02 ms
8954159 1223.75 ms 1236.84 ms 13.09 ms

App size

Revision Plain With Sentry Diff
b915d28 23.75 KiB 920.62 KiB 896.87 KiB
8954159 23.75 KiB 920.34 KiB 896.59 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@limbonaut limbonaut merged commit 285ff55 into main Aug 13, 2025
138 checks passed
@limbonaut limbonaut deleted the feat/add-frame-source-context branch August 13, 2025 11:30
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.

3 participants