fix(attachments): flush scope on updating attachments

[JoshuaMoelans]

Fixes #1933

Updates to attachments weren't flushed to the scope (only when adding it with sentry_attach_file). This caused updates to attachments to only apply on non-fatal events, since the native backend only knows about the state of the attachment from right before attaching it.

This fixes both questions in the original issue; attachment names are synced & so are the content_types, which initially made Sentry backend interpret the file as the default application/octet-stream which gets a Preview button (whereas the application/ztsd would not)

From the flow that UE hits on adding attachments, we see the same attach -> set_filename -> set_content_type flow that we now fix with scope syncing:

void FGenericPlatformSentrySubsystem::AddFileAttachment(TSharedPtr<ISentryAttachment> attachment)
{
	TSharedPtr<FGenericPlatformSentryAttachment> platformAttachment = StaticCastSharedPtr<FGenericPlatformSentryAttachment>(attachment);

	sentry_attachment_t* nativeAttachment =
		sentry_attach_file(TCHAR_TO_UTF8(*platformAttachment->GetPath()));

	if (!platformAttachment->GetFilename().IsEmpty())
		sentry_attachment_set_filename(nativeAttachment, TCHAR_TO_UTF8(*platformAttachment->GetFilename()));

	if (!platformAttachment->GetContentType().IsEmpty())
		sentry_attachment_set_content_type(nativeAttachment, TCHAR_TO_UTF8(*platformAttachment->GetContentType()));

	platformAttachment->SetNativeObject(nativeAttachment);

	attachments.Add(platformAttachment);
}

[codecov]

Codecov Report

❌ Patch coverage is 72.41379% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.79%. Comparing base (5bac907) to head (ba60934).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1934      +/-   ##
==========================================
+ Coverage   75.74%   75.79%   +0.05%     
==========================================
  Files          93       93              
  Lines       22128    22155      +27     
  Branches     3939     3945       +6     
==========================================
+ Hits        16760    16792      +32     
+ Misses       4484     4477       -7     
- Partials      884      886       +2     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JoshuaMoelans]

@sentry review

[JoshuaMoelans]

open q: both places where we now WITH_SCOPE are additional overhead, although these (probably) don't get called that often. This overhead is acceptable for the attachments that actually are on the global scope, but we also do it for those that are in the local scopes (or hint attachments).
We could walk the global attachments list to check if a scope lock is necessary, but this also seems like overhead (albeit less so than a full scope flush), so it might be worth considering.

[jpnurmi]

There are quite a few changes ahead if we want to do that, but I'd eventually

• replace heap-allocated sentry_attachment_t objects with sentry_value_t-backed value types,
• replace global scope mutex with fine-grained read-write locks, and
• replace backend flush functions with scope observers.

[jpnurmi]

I think this is fine for now.

There's an increasing number of reasons for moving towards sentry_value-based attachments and potentially getting rid of the whole backend flush-function altogether, but such changes would not happen overnight. This change helps until then.