-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
Comments
@lucas-zimerman i cant seem to reproduce this. does it still happen for you? |
I can reproduce it with latest 3.20.1, though with some minor changes:
|
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. |
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 Note the above two should mean changing 3 Throw when a null value is set |
assuming we go with 2. here is a PR #1932 |
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 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 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. |
Also I'll just add that it would have been a better design to encapsulate the |
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
Expected Result
An event sent to Sentry.
Actual Result
The text was updated successfully, but these errors were encountered: