fix(feedback): Align get user with JS implementation #4901

antonis · 2025-06-06T15:00:08Z

📢 Type of change

Bugfix
New feature
Enhancement
Refactoring

📜 Description

Align get user with JS implementation

💡 Motivation and Context

Fixes #4896

💚 How did you test it?

📝 Checklist

I added tests to verify 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.
All tests passing
No breaking changes

🔮 Next steps

github-actions · 2025-06-06T15:19:07Z

Android (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	440.14 ms	434.98 ms	-5.16 ms
Size	17.75 MiB	20.15 MiB	2.40 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`0e42017`	402.23 ms	415.04 ms	12.81 ms
`ec2a485`	450.84 ms	447.49 ms	-3.35 ms
`1f1c420`	403.32 ms	411.98 ms	8.66 ms
`6e8a851`	425.59 ms	433.51 ms	7.92 ms
`df5da5d`	425.55 ms	432.96 ms	7.41 ms
`b4d6bde`	425.51 ms	417.37 ms	-8.14 ms
`940bd65`	466.31 ms	458.52 ms	-7.79 ms
`7d3c3cb`	444.85 ms	456.65 ms	11.81 ms

App size

Revision	Plain	With Sentry	Diff
`0e42017`	17.75 MiB	20.15 MiB	2.40 MiB
`ec2a485`	17.75 MiB	20.15 MiB	2.40 MiB
`1f1c420`	17.75 MiB	20.15 MiB	2.40 MiB
`6e8a851`	17.75 MiB	20.15 MiB	2.40 MiB
`df5da5d`	17.75 MiB	20.15 MiB	2.40 MiB
`b4d6bde`	17.75 MiB	20.15 MiB	2.40 MiB
`940bd65`	17.75 MiB	20.15 MiB	2.40 MiB
`7d3c3cb`	17.75 MiB	20.15 MiB	2.40 MiB

Previous results on branch: antonis/feedback-getUser

Startup times

Revision	Plain	With Sentry	Diff
`8fe4d31`	437.13 ms	462.96 ms	25.82 ms

App size

Revision	Plain	With Sentry	Diff
`8fe4d31`	17.75 MiB	20.15 MiB	2.40 MiB

github-actions · 2025-06-06T15:26:18Z

Android (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	662.32 ms	715.56 ms	53.24 ms
Size	7.15 MiB	8.42 MiB	1.26 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`940bd65`+dirty	408.45 ms	419.75 ms	11.30 ms
`7d3c3cb`+dirty	395.20 ms	413.24 ms	18.04 ms
`0e42017`+dirty	387.33 ms	399.30 ms	11.97 ms
`ec2a485`+dirty	397.67 ms	390.91 ms	-6.76 ms
`df5da5d`+dirty	415.54 ms	456.96 ms	41.42 ms
`6e8a851`+dirty	403.44 ms	430.87 ms	27.43 ms
`b4d6bde`+dirty	390.51 ms	385.60 ms	-4.91 ms
`1f1c420`+dirty	383.31 ms	386.98 ms	3.67 ms

App size

Revision	Plain	With Sentry	Diff
`940bd65`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`7d3c3cb`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`0e42017`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`ec2a485`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`df5da5d`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`6e8a851`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`b4d6bde`+dirty	7.15 MiB	8.42 MiB	1.26 MiB
`1f1c420`+dirty	7.15 MiB	8.42 MiB	1.26 MiB

Previous results on branch: antonis/feedback-getUser

Startup times

Revision	Plain	With Sentry	Diff
`8fe4d31`+dirty	413.76 ms	435.16 ms	21.40 ms

App size

Revision	Plain	With Sentry	Diff
`8fe4d31`+dirty	7.15 MiB	8.42 MiB	1.26 MiB

github-actions · 2025-06-06T15:27:30Z

iOS (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1208.35 ms	1204.34 ms	-4.01 ms
Size	2.63 MiB	3.78 MiB	1.15 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`df5da5d`+dirty	1235.98 ms	1243.41 ms	7.43 ms
`6e8a851`+dirty	1227.96 ms	1235.61 ms	7.65 ms
`0e42017`+dirty	1225.89 ms	1231.63 ms	5.74 ms
`1f1c420`+dirty	1216.77 ms	1214.48 ms	-2.29 ms
`940bd65`+dirty	1216.88 ms	1225.23 ms	8.35 ms
`b4d6bde`+dirty	1223.22 ms	1243.56 ms	20.34 ms
`7d3c3cb`+dirty	1226.39 ms	1227.10 ms	0.71 ms
`ec2a485`+dirty	1219.72 ms	1224.66 ms	4.94 ms

App size

Revision	Plain	With Sentry	Diff
`df5da5d`+dirty	2.63 MiB	3.78 MiB	1.15 MiB
`6e8a851`+dirty	2.63 MiB	3.78 MiB	1.15 MiB
`0e42017`+dirty	2.63 MiB	3.78 MiB	1.15 MiB
`1f1c420`+dirty	2.63 MiB	3.77 MiB	1.14 MiB
`940bd65`+dirty	2.63 MiB	3.78 MiB	1.15 MiB
`b4d6bde`+dirty	2.63 MiB	3.77 MiB	1.14 MiB
`7d3c3cb`+dirty	2.63 MiB	3.78 MiB	1.15 MiB
`ec2a485`+dirty	2.63 MiB	3.78 MiB	1.15 MiB

Previous results on branch: antonis/feedback-getUser

Startup times

Revision	Plain	With Sentry	Diff
`8fe4d31`+dirty	1221.22 ms	1230.55 ms	9.33 ms

App size

Revision	Plain	With Sentry	Diff
`8fe4d31`+dirty	2.63 MiB	3.78 MiB	1.15 MiB

github-actions · 2025-06-06T15:30:41Z

iOS (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1233.47 ms	1233.82 ms	0.35 ms
Size	3.19 MiB	4.35 MiB	1.17 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`df5da5d`+dirty	1226.82 ms	1234.88 ms	8.06 ms
`6e8a851`+dirty	1222.57 ms	1223.67 ms	1.10 ms
`0e42017`+dirty	1235.77 ms	1247.43 ms	11.66 ms
`1f1c420`+dirty	1238.06 ms	1234.04 ms	-4.02 ms
`940bd65`+dirty	1224.39 ms	1215.57 ms	-8.82 ms
`b4d6bde`+dirty	1218.73 ms	1223.26 ms	4.53 ms
`7d3c3cb`+dirty	1214.56 ms	1234.53 ms	19.97 ms
`ec2a485`+dirty	1209.65 ms	1229.18 ms	19.53 ms

App size

Revision	Plain	With Sentry	Diff
`df5da5d`+dirty	3.19 MiB	4.35 MiB	1.17 MiB
`6e8a851`+dirty	3.19 MiB	4.35 MiB	1.17 MiB
`0e42017`+dirty	3.19 MiB	4.35 MiB	1.16 MiB
`1f1c420`+dirty	3.19 MiB	4.34 MiB	1.16 MiB
`940bd65`+dirty	3.19 MiB	4.35 MiB	1.16 MiB
`b4d6bde`+dirty	3.19 MiB	4.34 MiB	1.16 MiB
`7d3c3cb`+dirty	3.19 MiB	4.35 MiB	1.16 MiB
`ec2a485`+dirty	3.19 MiB	4.35 MiB	1.17 MiB

Previous results on branch: antonis/feedback-getUser

Startup times

Revision	Plain	With Sentry	Diff
`8fe4d31`+dirty	1228.45 ms	1230.10 ms	1.65 ms

App size

Revision	Plain	With Sentry	Diff
`8fe4d31`+dirty	3.19 MiB	4.35 MiB	1.17 MiB

# Conflicts: # packages/core/src/js/feedback/FeedbackWidget.tsx

krystofwoldrich · 2025-06-10T13:46:40Z

packages/core/src/js/feedback/FeedbackWidget.tsx


+  private _getUser = (): User | undefined => {
+    const currentUser = getCurrentScope().getUser();
+    if (currentUser && Object.keys(currentUser).length) {


Is there any specific edge case for which && Object.keys(currentUser).length is needed?

My thinking is if there is a user set (empty, or without email and name) we should take it.

Good point @krystofwoldrich 👍
I just used the JS logic on this but I agree with you and I'll iterate on this.

Fixed with beaa541

An alternative would be to check just the user email or name like:

if (currentUser && (currentUser.email || currentUser.name)) {

CHANGELOG.md

Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>

lucas-zimerman · 2025-06-11T20:16:53Z

packages/core/src/js/feedback/FeedbackWidget.tsx

+    const currentUser = getCurrentScope().getUser();
+    if (currentUser) {
+      return currentUser;
+    }
+    const isolationUser = getIsolationScope().getUser();
+    if (isolationUser) {
+      return isolationUser;
+    }


i: Most people set the user by using Sentry.setUser, with that we could invert the currentScope with isolationScope so it return in most of the cases on first try.

Suggested change

const currentUser = getCurrentScope().getUser();

if (currentUser) {

return currentUser;

}

const isolationUser = getIsolationScope().getUser();

if (isolationUser) {

return isolationUser;

}

const isolationUser = getIsolationScope().getUser();

if (isolationUser {

return isolationUser;

}

const currentUser = getCurrentScope().getUser();

if (currentUser) {

return currentUser;

}

Your suggestion makes sense to me @lucas-zimerman and also aligns with the documentation given that we are handling user data. On the other hand any data set on the current scope should take precedence over data set on the isolation and global scope, which is the logic used in the JS feedback widget 🤔

we can skip the suggeston and talk about it on a follow up issue so the main issue doesn't get blocked for too long.
To me it sounds like, on react native context, where you'll only have one user, Sentry.SetUser should use the global scope instead of an isolation scope.

Maybe it could be a nice nit that we could change on V7 if others agree.

Opened an issue for this #4915

lucas-zimerman

LGTM!

krystofwoldrich

LGTM 🚀 Thank you!

antonis added 2 commits June 6, 2025 17:57

fix(feedback): Align get user with JS implementation

f9f8d6d

Fix lint issue

ddb784d

antonis added 4 commits June 9, 2025 10:17

Merge branch 'main' into antonis/feedback-getUser

7a3a6a1

# Conflicts: # packages/core/src/js/feedback/FeedbackWidget.tsx

Adds changelog

bbacab5

Optimise scope usage

a8e4ab1

Adds tests

974f698

antonis marked this pull request as ready for review June 9, 2025 09:30

antonis requested review from krystofwoldrich and lucas-zimerman as code owners June 9, 2025 09:30

krystofwoldrich reviewed Jun 10, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

antonis and others added 4 commits June 10, 2025 16:52

Update CHANGELOG.md

2287e25

Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>

Use current or isolation user is set without checks

beaa541

Merge branch 'main' into antonis/feedback-getUser

10bf2ae

Fix changelog

68f9726

antonis requested a review from krystofwoldrich June 10, 2025 14:43

lucas-zimerman reviewed Jun 11, 2025

View reviewed changes

lucas-zimerman approved these changes Jun 12, 2025

View reviewed changes

antonis mentioned this pull request Jun 12, 2025

Consider using the Global scope for static APIs (like setUser...) #4915

Open

krystofwoldrich approved these changes Jun 12, 2025

View reviewed changes

antonis merged commit c352c30 into main Jun 13, 2025
107 of 111 checks passed

antonis deleted the antonis/feedback-getUser branch June 13, 2025 04:27

Uh oh!

fix(feedback): Align get user with JS implementation #4901

fix(feedback): Align get user with JS implementation #4901

Uh oh!

Conversation

antonis commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📢 Type of change

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Android (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/feedback-getUser

Startup times

App size

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Android (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/feedback-getUser

Startup times

App size

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

iOS (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/feedback-getUser

Startup times

App size

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

iOS (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/feedback-getUser

Startup times

App size

Uh oh!

krystofwoldrich Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

antonis Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

antonis Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lucas-zimerman Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonis Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

lucas-zimerman Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonis Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

lucas-zimerman left a comment

Choose a reason for hiding this comment

antonis commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

lucas-zimerman Jun 11, 2025 •

edited

Loading

lucas-zimerman Jun 12, 2025 •

edited

Loading