Inconsistent event schemas between SDKs (JS / Python)

[alexmojaki]

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.9.0

Steps to Reproduce

Background: I use sentry_sdk in Python running in the browser with Pyodide to collect a serialised Sentry event about a Python exception without actually sending it, because Python code for sending network requests doesn't work in the browser. The code for that is here: https://github.com/alexmojaki/futurecoder/blob/e09f016bd69f893d3eb81e1a4bfa804953aedb4e/core/utils.py#L205-L225 . The event is returned to JS which sends it to the server using Sentry.captureEvent. I'm @sentry/react, and it and all its dependencies like @sentry/browser are on version 7.8.1.

In summary:

1. In Python, use sentry_sdk.init() to set a custom transport to intercept events.
2. Then sentry_sdk.capture_exception() to collect an event, but don't send it.
3. Pass the captured event to Sentry.captureEvent() in JS.

Expected Result

The event is captured successfully and can be viewed on sentry.io. This has in fact been working great for a while, and the UI shows me info from both the JS and Python SDKs in one event.

Actual Result

Sentry.captureEvent() in JS runs into a TypeError: (e.breadcrumbs || []) is not iterable, which itself actually gets captured by Sentry and sent instead:

The problem is that in the Python event, breadcrumbs is a dict with a key values mapping to a list. But JS expects breadcrumbs to just be the list (array) rather than a dict (object). If I set event.breadcrumbs = event.breadcrumbs.values in JS, then everything goes back to working perfectly. However this doesn't mean much since in this case event.breadcrumbs is just {values: []}. I don't know if there are sometimes keys other than values, or if the lists themselves are also sometimes incompatible between the SDKs.

I don't know to what extent the different SDKs are meant to have compatible schema, but it seems like it's been working well so far and it'd be a pity to lose that. Also, I don't know if this issue belongs here or in a JS repo.

[sl0thentr0py]

hey @alexmojaki this is a really unconventional use case, but bonus points for creativity! I'm surprised the rest of the SDK works at all since we never tested in webassembly before. ^^

I don't know to what extent the different SDKs are meant to have compatible schema, but it seems like it's been working well so far and it'd be a pity to lose that. Also, I don't know if this issue belongs here or in a JS repo.

Yes, there might be some inconsistencies between SDKs in this regard, because our event schema allows both a direct list of objects or nested under values as noted for exception here. The same holds for breadcrumbs.

The exception attribute should be an object with the attribute values containing a list of one or more values that are objects in the format described below. Alternatively, the exception attribute may be a flat list of objects in the format below.

Unfortunately, this really counts as an unsupported use case so you'll just have to deal with the inconsistencies on your end, sorry! If you need me to confirm something about the schema, you can ask me here again, but I'll close this issue and move it to a discussion.

[alexmojaki]

I'm surprised the rest of the SDK works at all since we never tested in webassembly before. ^^

Pyodide is great, you can expect any pure Python code that works in CPython to work perfectly in Pyodide, even if it relies heavily on CPython implementation details, until of course it has to interact with the outside world like send a network request.

Yes, there might be some inconsistencies between SDKs in this regard, because our event schema allows both a direct list of objects or nested under values as noted for exception here. The same holds for breadcrumbs.

Thanks for the explanation. It seems then that there a few issues that are reasonably worth fixing:

1. The schema doc for breadcrumbs should have a similar note to the one about exception. Looking at the doc, there's no way to know that breadcrumbs is allowed to be an array, and the JS SDK appears to be in violation of the schema.
2. The machine-readable type definitions for both breadcrumbs and exception should reflect the description for humans, e.g. breadcrumbs: null | Breadcrumbs | Array<Breadcrumb | null>.
3. The JS SDK code should correctly reflect the schema, e.g. in https://github.com/getsentry/sentry-javascript/blob/9b7131fcbf7f47be3a78893fa56a70da6c29ee7a/packages/types/src/event.ts#L39 it should be something like breadcrumbs?: Breadcrumb[] | {values: Breadcrumb[]};

Unfortunately, this really counts as an unsupported use case so you'll just have to deal with the inconsistencies on your end, sorry!

The use case doesn't seem to be the most relevant thing here. It's clear just from the existence of pages like https://develop.sentry.dev/sdk/event-payloads/types/ that Sentry cares about types and schemas, and I think getting this stuff right is worth it. The use case is an example of why these schemas are worth it, because the two SDKs have fit together for some time and still very nearly do, producing benefits that you didn't even anticipate. I think this is a thing of beauty worth being proud of and preserving:

I also think it's a use case worth supporting at least a little. It's becoming increasingly common for languages like Python to be used in the browser, it's still just as useful as always to use Sentry to monitor errors in those cases, and it doesn't require unusual effort from either users or Sentry. It would be unreasonable to ask you to start adding code and tests specifically for webassembly, but this isn't close to that, it's just a matter of the JS SDK correctly following the official schema.

The use case is also much more general than 'Python in the browser'. For example, I've considered reusing this exact technique at my work to start monitoring our Python code with Sentry. The Python code runs in a server, but in a secure sandbox that has no network access, so it would send the event to the parent process running NodeJS. I expect this would work exactly as well as the browser case, i.e. fully apart from the breadcrumbs issue. I also imagine the result would be similar with other languages instead of Python, as long as that SDK also produces an object with values instead of an array.

I don't personally need this fix, since the workaround is simple and I've verified that it works: alexmojaki/futurecoder@4b6a75d

But I think that this tiny inconsistency in what seems to be otherwise perfect compatibility is tragic. It seems likely that other people will try to do similar things, run into the same error, and conclude that it's impossible, without having any way of knowing how close they were.

[sl0thentr0py]

that's some great feedback @alexmojaki
I'm editing the title to reflect this discussion better.
And we'll follow up on making the docs accurate.

[AbhiPrasad]

Thank you for the feedback!

You bring up a great point with the consistency being important - but unfortunately, changing the per SDK schema will end up being a breaking change (and we need to decide if we settle with values or a flat list). As such, I don't think it's feasible for us to change this immediately. The reason this is breaking is because users might be expecting certain values in the event schema internally (to filter in beforeSend or write custom integrations/transports for).

What we can do is to transfer this discussion onto an issue in the develop repo, and start collecting thoughts from various SDK maintainers: https://github.com/getsentry/develop/issues. I can take care of that! Then we make make a decision on this, and then slowly change all the SDKs as we bump major versions.

The JS SDK code should correctly reflect the schema

As a side note, the flat list is more bundle size efficient, so it's hard to justify using values here. Our event ingestion system relay ends up transforming everything to be a flat list under values.

[AbhiPrasad]

Added here: getsentry/develop#662

[antonpirker]

The Python SDK could just change the breadcrumbs after beforeSend and before they event is actually sent by the transport. Adding some sort of compatibility layer. (I know this again ads overhead to the Python SDK, which we do not want to have, ...)