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.

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

Scope removeContext and removeTag not working for app crash #2588

Open
denrase opened this issue Jan 9, 2023 · 8 comments
Open

Scope removeContext and removeTag not working for app crash #2588

denrase opened this issue Jan 9, 2023 · 8 comments

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

@philipphofmann
Copy link
Member

The problem is here

- (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.

@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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

6 participants