Skip to content

Use file manager of dependency container for SentryClient #5296

@philprime

Description

@philprime

Description

I noticed today that we have a SentryFileManager in the SentryDependencyContainer which is used for most cases, but not for the SentryClient because it initializes it's own file manager in SentrySDK:

SentryClient *newClient = [[SentryClient alloc] initWithOptions:options];
[newClient.fileManager moveAppStateToPreviousAppState];
[newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs];

/** Internal constructor for testing purposes. */
- (nullable instancetype)initWithOptions:(SentryOptions *)options
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue
deleteOldEnvelopeItems:(BOOL)deleteOldEnvelopeItems
{
NSError *error;
SentryFileManager *fileManager = [[SentryFileManager alloc] initWithOptions:options
dispatchQueueWrapper:dispatchQueue
error:&error];
if (error != nil) {
SENTRY_LOG_FATAL(@"Failed to initialize file system: %@", error.localizedDescription);
return nil;
}
return [self initWithOptions:options
fileManager:fileManager
deleteOldEnvelopeItems:deleteOldEnvelopeItems];
}

It got introduced here before being last changed here.

Found this while working on this PR feedback comment as moveContextFileToPreviousContextFile is invoked from SentrySDK.

We should reuse the file manager of the dependency container so that the client uses the same as all other components. Right now the file manager in the dependency container is the one created for SentrySDK.options, and even if the options change, the file manager does not, leading to internal inconsistencies.

The SDK resets the dependency container on close. This is only a problem if you call start with a different DSN multiple times in a row. And if you don't use the static API. This could also be a reason with flaky tests not cleaning up correctly.

Image
Image

Metadata

Metadata

Assignees

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions