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

invalid event envelope. Error causes: unexpected end of file. If Context is null #1332

Closed
lucas-zimerman opened this issue Nov 19, 2021 · 7 comments · Fixed by #1942
Closed
Assignees
Labels
Bug Something isn't working

Comments

@lucas-zimerman
Copy link
Collaborator

Environment

How do you use Sentry?
SaaS and Self Hosted.

Which SDK and version?
latest.

Steps to Reproduce

Only happens with the caching feature.

run the snippet

using System;
using System.Globalization;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Sentry;
using Sentry.Infrastructure;

SentrySdk.Init(o =>
{
    o.Dsn = "https://eb18e953812b41c3aeb042e666fd3b5c@o447951.ingest.sentry.io/5428537";
    o.Debug = true;
    o.DiagnosticLogger = new TraceDiagnosticLogger(SentryLevel.Debug); //only used for revealing the data on the IDE.
    o.CacheDirectoryPath = "D:/bob";
});


SentrySdk.ConfigureScope(scope =>
{
    scope.Contexts["MyCoolContext"] = null;
    scope.Contexts["MyCoolContext2"] = null;
});
SentrySdk.CaptureMessage("Hello Wôrld");
await Task.Delay(1000);

Expected Result

An event sent to Sentry.

Actual Result

  Error: Sentry rejected the envelope 3e2862cff51d43b3860e4513ef522764. Status code: BadRequest. Error detail: invalid event envelope. Error causes: unexpected end of file.
  Debug: Failed envelope '3e2862cff51d43b3860e4513ef522764' has payload:
{"sdk":{"name":"Sentry","version":"3.12.0-alpha.1"},"event_id":"3e2862cff51d43b3860e4513ef522764"}
{"type":"event","length":1965}
{"modules":{"System.Private.CoreLib":"5.0.0.0","Sentry.Samples.Console.Basic":"9.8.7.0","System.Runtime":"5.0.0.0","Sentry":"3.12.0.0","System.Net.Primitives":"5.0.0.0","System.IO.Compression":"5.0.0.0","System.Private.Uri":"5.0.0.0","System.Diagnostics.Process":"5.0.0.0","System.ComponentModel.Primitives":"5.0.0.0","Microsoft.Win32.Primitives":"5.0.0.0","System.Runtime.InteropServices":"5.0.0.0","System.Threading":"5.0.0.0","System.Diagnostics.TraceSource":"5.0.0.0","System.Net.Http":"5.0.0.0","System.Diagnostics.Tracing":"5.0.0.0","System.Diagnostics.DiagnosticSource":"5.0.0.0","System.Net.Security":"5.0.0.0","System.Security.Cryptography.X509Certificates":"5.0.0.0","System.Collections":"5.0.0.0","System.Collections.Concurrent":"5.0.0.0","System.Security.Cryptography.Algorithms":"5.0.0.0","System.Security.Cryptography.Primitives":"5.0.0.0","System.Memory":"5.0.0.0","System.IO.FileSystem":"5.0.0.0","System.Linq":"5.0.0.0","System.Threading.Thread":"5.0.0.0","System.Diagnostics.StackTrace":"5.0.0.0"},"event_id":"3e2862cff51d43b3860e4513ef522764","timestamp":"2021-11-19T21:00:13.8001629+00:00","logentry":{"message":"Hello W\u00F4rld"},"platform":"csharp","release":"Sentry.Samples.Console.Basic@9.8.7","level":"info","request":{},"contexts":{"app":{"type":"app","app_start_time":"2021-11-19T21:00:12.4422617+00:00"},"device":{"type":"device","timezone":"E. South America Standard Time","timezone_display_name":"(UTC-03:00) Brasilia","boot_time":"2021-11-17T17:50:47.9937672+00:00"},"runtime":{"type":"runtime","name":".NET","version":"5.0.10","raw_description":".NET 5.0.10"},"Current Culture":{"Name":"en-US","DisplayName":"English (United States)","Calendar":"GregorianCalendar"},"os":{"type":"os","raw_description":"Microsoft Windows 10.0.22000"}},"user":{},"environment":"debug","sdk":{"packages":[{"name":"nuget:Sentry","version":"3.12.0-alpha.1"}],"name":"sentry.dotnet","version":"3.12.0-alpha.1"}}

@lucas-zimerman lucas-zimerman added the Bug Something isn't working label Nov 19, 2021
@SimonCropp
Copy link
Contributor

@lucas-zimerman i cant seem to reproduce this. does it still happen for you?

@mattjohnsonpint
Copy link
Contributor

I can reproduce it with latest 3.20.1, though with some minor changes:

    o.Dsn = "https://eb18e953812b41c3aeb042e666fd3b5c@o447951.ingest.sentry.io/5428537";
    o.Debug = true;
    o.CacheDirectoryPath = "bin/temp";
  Error: Sentry rejected the envelope 825ebfa763dd420a99243523a0165d74. Status code: BadRequest. Error detail: invalid event envelope. Error causes: unexpected end of file.
  Debug: Failed envelope '825ebfa763dd420a99243523a0165d74' has payload: {"sdk":{"name":"sentry.dotnet","version":"3.20.1"},"event_id":"825ebfa763dd420a99243523a0165d74","sent_at":"2022-08-04T15:05:38.072424+00:00"} {"type":"event","length":3239} {"modules":{"System.Private.CoreLib":"6.0.0.0","ConsoleApp16":"1.0.0.0","System.Runtime":"6.0.0.0","JetBrains.Microsoft.Extensions.DotNetDeltaApplier":"1.0.0.0","System.Linq":"6.0.0.0","System.Collections":"6.0.0.0","System.Collections.Concurrent":"6.0.0.0","Sentry":"3.20.1.0","System.Net.Primitives":"6.0.0.0","System.IO.Compression":"6.0.0.0","System.IO.Pipes":"6.0.0.0","System.Console":"6.0.0.0","System.Threading":"6.0.0.0","Microsoft.Win32.Primitives":"6.0.0.0","System.Net.Sockets":"6.0.0.0","System.Diagnostics.Tracing":"6.0.0.0","System.Threading.ThreadPool":"6.0.0.0","System.Runtime.InteropServices.RuntimeInformation":"6.0.0.0","System.Threading.Thread":"6.0.0.0","System.Memory":"6.0.0.0","System.Runtime.Loader":"6.0.0.0","System.Diagnostics.Process":"6.0.0.0","System.Private.Uri":"6.0.0.0","System.ComponentModel.Primitives":"6.0.0.0","System.Net.Http":"6.0.0.0","System.Diagnostics.DiagnosticSource":"6.0.0.0","System.Net.Security":"6.0.0.0","System.Security.Cryptography.X509Certificates":"6.0.0.0","System.Security.Cryptography.Algorithms":"6.0.0.0","System.Security.Cryptography.Primitives":"6.0.0.0","System.Net.Requests":"6.0.0.0","System.Text.Json":"6.0.0.0","System.Numerics.Vectors":"6.0.0.0","System.Runtime.CompilerServices.Unsafe":"6.0.0.0","System.Text.Encoding.Extensions":"6.0.0.0","System.ObjectModel":"6.0.0.0","System.Text.Encodings.Web":"6.0.0.0","System.Runtime.Intrinsics":"6.0.0.0","System.Runtime.InteropServices":"6.0.0.0","System.Net.NameResolution":"6.0.0.0","System.Collections.NonGeneric":"6.0.0.0","System.Security.Cryptography.Encoding":"6.0.0.0","System.Formats.Asn1":"6.0.0.0","System.Runtime.Numerics":"6.0.0.0","System.Diagnostics.StackTrace":"6.0.0.0"},"event_id":"825ebfa763dd420a99243523a0165d74","timestamp":"2022-08-04T15:05:37.940945+00:00","logentry":{"message":"Hello W\u00F4rld"},"platform":"csharp","release":"ConsoleApp16@1.0.0","level":"info","request":{},"contexts":{"Current Culture":{"name":"en-US","display_name":"English (United States)","calendar":"GregorianCalendar"},"ThreadPool Info":{"min_worker_threads":10,"min_completion_port_threads":10,"max_worker_threads":32767,"max_completion_port_threads":1000,"available_worker_threads":32766,"available_completion_port_threads":1000},"Dynamic Code":{"Compiled":true,"Supported":true},"os":{"type":"os","raw_description":"Darwin 21.5.0 Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:37 PDT 2022; root:xnu-8020.121.3~4/RELEASE_ARM64_T6000"},"device":{"type":"device","timezone":"America/Los_Angeles","timezone_display_name":"(UTC-08:00) Pacific Time (Los Angeles)","boot_time":"2022-07-24T03:18:01.8133283+00:00"},"runtime":{"type":"runtime","name":".NET","version":"6.0.7","raw_description":".NET 6.0.7","identifier":"osx.12-arm64"},"Memory Info":{"allocated_bytes":2242296,"high_memory_load_threshold_bytes":30923764531,"total_available_memory_bytes":34359738368,"finalization_pending_count":0,"compacted":false,"concurrent":false,"pause_durations":[0,0]},"app":{"type":"app","app_start_time":"2022-08-04T15:05:37.28985+00:00"}},"user":{},"environment":"production","sdk":{"packages":[{"name":"nuget:sentry.dotnet","version":"3.20.1"}],"name":"sentry.dotnet","version":"3.20.1"}}

@mattjohnsonpint
Copy link
Contributor

It would appear that the null entries are removed from the context section of the payload when caching is enabled, and the length header is not updated.

We should ensure first that the length header is always correct, but also we should decide whether to either send null context info always or never. It shouldn't vary based on caching.

@SimonCropp SimonCropp self-assigned this Sep 18, 2022
@SimonCropp
Copy link
Contributor

The server-side accepts nulls for context values. but they are not show in the UI

@mattjohnsonpint @bruno-garcia can you check if the above is by design? Seems to me we should never hide information about an event??

Assuming it is by design, should we:

1 Accept and ignore null
2 Accept and send nulls.

Note the above two should mean changing Contexts to inherit from a ConcurrentDictionary<string, object?>. given with nullable reference types enabled the above sample will not compile

3 Throw when a null value is set

@SimonCropp
Copy link
Contributor

assuming we go with 2. here is a PR #1932

@mattjohnsonpint
Copy link
Contributor

mattjohnsonpint commented Sep 19, 2022

On further thought - Null contexts don't make much sense. Context is extra information at the time of the event, such as the OS name or the amount of memory available, etc.

We define Contexts as extending from ConcurrentDictionary<string, object>, so if you use it in code where nullability checks are enabled (<Nullable>enable</Nullable>) the compiler will give a warning:

image

image

The problem is that one could easily be using Sentry without having nullability checks enabled. So basically, any public API that exposes a class parameter has to deal with the possibility of the user passing null, even if it's been marked as not-null.

SO - In this case, we want to not accept nulls, and also ignore them. (variation on option 1)

Throwing (option 3) would be ok in some apps, but not here because we have a general principal of Sentry not itself being the cause of an issue. Say a user was passing in custom context, and it usually was not null. Throwing on null would surface to the end-user, and would prevent the error from being sent to Sentry. So, better to silently ignore.

In summary - the API surface shouldn't change here, we should just make sure we filter out nulls from context values and the length header should correctly reflect that appropriately so the envelope sends without failure.

@mattjohnsonpint
Copy link
Contributor

mattjohnsonpint commented Sep 19, 2022

Also I'll just add that it would have been a better design to encapsulate the ConcurrentDictionary rather than extending from it. Contexts : IDictionary<string, object> would have allowed us to filter out nulls when data was being added, rather than afterwards. It would have also hidden the concurrentness, which is an implementation detail. But changing it now would be a breaking change, so perhaps for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants