Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Scope removeContext and removeTag not working for app crash #2588

Open
denrase opened this issue Jan 9, 2023 · 10 comments · May be fixed by #4960
Open

Scope removeContext and removeTag not working for app crash #2588

denrase opened this issue Jan 9, 2023 · 10 comments · May be fixed by #4960
Assignees
@philprime
Labels
Platform: Cocoa Type: Bug

Comments

@denrase
Copy link
Collaborator

denrase commented Jan 9, 2023

Platform

iOS

Installed

Swift Package Manager

Version

8.0.0-beta4

Steps to Reproduce

  1. Set a context key with data and remove it again.
SentrySDK.configureScope { scope in
    scope.setContext(value: ["key": "value"], key: "context-key")
    scope.setContext(value: ["key-2": "value-2"], key: "context-key-2")
    scope.removeContext(key: "context-key-2")
}
  1. Launch application and trigger a hard crash. This was done with the swift sample app.
  2. Launch application again in order for the crash to sync to the backend.

Same behaviour for set/remove tag.

Expected Result

Only data for context-key is present.

Actual Result

Data for context-key-2 is still present.

Bildschirmfoto 2023-01-09 um 14 01 36
@denrase denrase mentioned this issue Jan 9, 2023
Merged
5 tasks
@philipphofmann
Copy link
Member

The problem is here

sentry-cocoa/Sources/Sentry/SentryScope.m

Lines 226 to 235 in e6fd1fb

- (void)removeContextForKey:(NSString *)key
{
@synchronized(_contextDictionary) {
[_contextDictionary removeObjectForKey:key];
for (id<SentryScopeObserver> observer in self.observers) {
[observer setExtras:_contextDictionary];
}
}
}

It should be setContext instead of setExtras.

@github-project-automation github-project-automation bot moved this to Needs Discussion in Mobile & Cross Platform SDK Jan 9, 2023
@marandaneto
Copy link
Contributor

@philipphofmann can we fix that and cut a patch for v7? Flutter v7 with Cocoa v8 needs more time.

@philipphofmann
Copy link
Member

@philipphofmann can we fix that and cut a patch for v7? Flutter v7 with Cocoa v8 needs more time.

Is it that important? Of course, we can, it's just extra work.

@marandaneto
Copy link
Contributor

It's a bug that affects the Hybrid SDKs + the iOS SDK, it's not a p0 but definitely, a bug, imagine that you are trying to remove some PII and you can't, then it's more severe.

@brustolin brustolin moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Jan 11, 2023
@brustolin
Copy link
Contributor

Since this looks like an edge case that we don't have any complains from customer, we gonna fix this only for v8. Unless someone has a strong opinion against it.

@philipphofmann
Copy link
Member

imagine that you are trying to remove some PII and you can't, then it's more severe.

If you once add any PII to the context or the tags and you remove it directly afterward, it still could end up in Sentry. Therefore, the PII argument is weaker for me. You should never add any PII to the context or tags.

@marandaneto
Copy link
Contributor

imagine that you are trying to remove some PII and you can't, then it's more severe.

If you once add any PII to the context or the tags and you remove it directly afterward, it still could end up in Sentry. Therefore, the PII argument is weaker for me. You should never add any PII to the context or tags.

I agree with the statement that you should not add PII, but IMO that's another problem, one can add it already, so one should be able to remove it, I'm focused on the side effect of the bug and not the intention of the user.

@philipphofmann
Copy link
Member

I agree with the statement that you should not add PII, but IMO that's another problem, one can add it already, so one should be able to remove it, I'm focused on the side effect of the bug and not the intention of the user.

That's a good point, but we don't think we must do a hotfix. We definitely are going to fix this, but we think it's sufficient to do this in 8.x.x.

@philprime
Copy link
Contributor

This issue has not had any activity in 2 years. Is this still an issue in the latest v8.x.x releases?

@philipphofmann
Copy link
Member

The problem still exists. The removeContextForKey didn't change in the Scope. This should be an easy fix.

@philprime philprime linked a pull request Mar 7, 2025 that will close this issue
Open
7 tasks
@philprime philprime self-assigned this Mar 7, 2025
@philprime philprime moved this from Backlog to Needs Review in Mobile & Cross Platform SDK Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@philprime philprime

Labels
Platform: Cocoa Type: Bug
Projects
Mobile & Cross Platform SDK
Status: Needs Review
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Fix removing value from context not updating observer context getsentry/sentry-cocoa
7 participants
@kahest @philipphofmann @denrase @marandaneto @hubertdeng123 @brustolin @philprime