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

URGENT: Fix events rejected due to duplicate sent_at header #1818

Merged
merged 4 commits into from
Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

### Fixes

- URGENT: Fix events rejected due to duplicate `sent_at` header when offline caching is enabled through `CacheDirectoryPath` ([#1818](https://github.com/getsentry/sentry-dotnet/pull/1818))
- Fix null ref in aspnet TryGetTraceHeader ([#1807](https://github.com/getsentry/sentry-dotnet/pull/1807))


## 3.20.0

### Features
Expand Down
21 changes: 16 additions & 5 deletions src/Sentry/Envelopes/Envelope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ private async Task SerializeHeaderAsync(
ISystemClock clock,
CancellationToken cancellationToken)
{
var headerItems = Header.Append(SentAtKey, clock.GetUtcNow());
// Append the sent_at header, except when writing to disk
var headerItems = !stream.IsFileStream()
? Header.Append(SentAtKey, clock.GetUtcNow())
: Header;

var writer = new Utf8JsonWriter(stream);

#if NET461 || NETSTANDARD2_0
Expand All @@ -72,14 +76,16 @@ private async Task SerializeHeaderAsync(

private void SerializeHeader(Stream stream, IDiagnosticLogger? logger, ISystemClock clock)
{
var headerItems = Header.Append(SentAtKey, clock.GetUtcNow());
// Append the sent_at header, except when writing to disk
var headerItems = !stream.IsFileStream()
? Header.Append(SentAtKey, clock.GetUtcNow())
: Header;

using var writer = new Utf8JsonWriter(stream);
writer.WriteDictionaryValue(headerItems, logger);
writer.Flush();
}

// Gets the header and adds a sent_at timestamp

/// <inheritdoc />
public Task SerializeAsync(
Stream stream,
Expand Down Expand Up @@ -270,9 +276,14 @@ internal static Envelope FromClientReport(ClientReport clientReport)
prevByte = curByte;
}

return
var header =
Json.Parse(buffer.ToArray(), JsonExtensions.GetDictionaryOrNull)
?? throw new InvalidOperationException("Envelope header is malformed.");

// The sent_at header should not be included in the result
header.Remove(SentAtKey);

return header;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/Internal/Extensions/JsonExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static void Deconstruct(this JsonProperty jsonProperty, out string name,
value = jsonProperty.Value;
}

public static IReadOnlyDictionary<string, object?>? GetDictionaryOrNull(this JsonElement json)
public static Dictionary<string, object?>? GetDictionaryOrNull(this JsonElement json)
{
if (json.ValueKind != JsonValueKind.Object)
{
Expand All @@ -46,7 +46,7 @@ public static void Deconstruct(this JsonProperty jsonProperty, out string name,
return result;
}

public static IReadOnlyDictionary<string, string?>? GetStringDictionaryOrNull(this JsonElement json)
public static Dictionary<string, string?>? GetStringDictionaryOrNull(this JsonElement json)
{
if (json.ValueKind != JsonValueKind.Object)
{
Expand Down
3 changes: 3 additions & 0 deletions src/Sentry/Internal/Extensions/StreamExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,8 @@ public static async Task WriteByteAsync(
return null;
}
}

public static bool IsFileStream(this Stream? stream) =>
stream is FileStream || stream?.GetType().Name == "MockFileStream";
}
}
4 changes: 3 additions & 1 deletion test/Sentry.Testing/MockClock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

public class MockClock : ISystemClock
{
private readonly DateTimeOffset _value;
private DateTimeOffset _value;

public MockClock(DateTimeOffset value) => _value = value;

Expand All @@ -11,4 +11,6 @@ public MockClock() : this(DateTimeOffset.MaxValue)
}

public DateTimeOffset GetUtcNow() => _value;

public void SetUtcNow(DateTimeOffset value) => _value = value;
}
32 changes: 32 additions & 0 deletions test/Sentry.Tests/Internals/Http/CachingTransportTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.IO.Abstractions.TestingHelpers;
using System.Net;
using System.Net.Http;
using System.Net.Sockets;
Expand Down Expand Up @@ -677,4 +678,35 @@ public async Task TransportResumesWhenNetworkComesBackOnline()
var envelopes = innerTransport.GetSentEnvelopes();
envelopes.Should().NotBeEmpty();
}

[Fact]
public async Task DoesntWriteSentAtHeaderToCacheFile()
{
// Arrange
using var cacheDirectory = new TempDirectory(_fileSystem);
var options = new SentryOptions
{
Dsn = ValidDsn,
DiagnosticLogger = _logger,
Debug = true,
CacheDirectoryPath = cacheDirectory.Path,
FileSystem = _fileSystem
};

var innerTransport = Substitute.For<ITransport>();
await using var transport = CachingTransport.Create(innerTransport, options, startWorker: false);

using var envelope = Envelope.FromEvent(new SentryEvent());

// Act
await transport.SendEnvelopeAsync(envelope);

// Assert
var filePath = _fileSystem
.EnumerateFiles(cacheDirectory.Path, "*", SearchOption.AllDirectories)
.Single();

var contents = _fileSystem.ReadAllTextFromFile(filePath);
Assert.DoesNotContain("sent_at", contents);
}
}
29 changes: 28 additions & 1 deletion test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public class EnvelopeTests
// https://develop.sentry.dev/sdk/envelopes/#full-examples

private readonly IDiagnosticLogger _testOutputLogger;
private readonly ISystemClock _fakeClock;
private readonly MockClock _fakeClock;

public EnvelopeTests(ITestOutputHelper output)
{
Expand Down Expand Up @@ -698,4 +698,31 @@ value is IReadOnlyDictionary<string, string> nested &&
nested["version"] == SdkVersion.Instance.Version;
}).Should().BeTrue();
}

[Fact]
public async Task Serialization_RoundTrip_ReplacesSentAtHeader()
{
// Arrange
using var envelope = new Envelope(
new Dictionary<string, object> { ["event_id"] = "12c2d058d58442709aa2eca08bf20986" },
Array.Empty<EnvelopeItem>());

// Act
_fakeClock.SetUtcNow(DateTimeOffset.MinValue);
var serialized = await envelope.SerializeToStringAsync(_testOutputLogger, _fakeClock);

using var stream = new MemoryStream();
using var writer = new StreamWriter(stream);
await writer.WriteAsync(serialized);
await writer.FlushAsync();
stream.Seek(0, SeekOrigin.Begin);
var deserialized = await Envelope.DeserializeAsync(stream);

_fakeClock.SetUtcNow(DateTimeOffset.MaxValue);
var output = await deserialized.SerializeToStringAsync(_testOutputLogger, _fakeClock);

// Assert
output.Should().Be(
"{\"event_id\":\"12c2d058d58442709aa2eca08bf20986\",\"sent_at\":\"9999-12-31T23:59:59.9999999+00:00\"}\n");
}
}