Opaque with_scope behaviour with exceptions in block

[sl0thentr0py]

Creating this issue for further discussion.

If you do the following, the exception will be captured but the tag won't be used (because that scope is thrown away in an ensure).

    Sentry.with_scope do |scope|
      scope.set_tags(my_tag: "my value")
      1 / 0
    end

If you do the following, the exception will be captured and the tag will be visible.

    Sentry.with_scope do |scope|
      scope.set_tags(my_tag: "my value")

      begin
        1 / 0
      rescue => e
        Sentry.capture_exception(e)
      end
    end

Originally posted by @sl0thentr0py in https://github.com/getsentry/sentry-docs/pull/5328#issuecomment-1194148134

[st0012]

I think this is counter-intuitive and probably should be considered a bug.

[st0012]

After testing this, I think the actual problem is: should with_scope capture exceptions?

If it should, then we can

1. report the exception with the inner scope's data
2. pop the scope
3. re-raise the exception
4. the rest of the program continues with the original scope

(Because we now flag captured exceptions, it won't cause duplicated reporting)

If it shouldn't, we need to not popping the scope so other integrations (e.g. Rack middleware) can capture it with the inner scope's data. However, this can lead to other issues like polluting the request transaction

Ofc, we can also just keep the current behavior.

[sl0thentr0py]

@st0012 so there are some things to consider here.

1. with_scope belongs to the Unified API so we can't just change it outright, even though I personally don't like these semantics.
2. we could introduce a new API that does the better semantics (capture/pop/re-raise) as you said, maybe call it try_with_scope (name could be better potentially)
3. if people like that more, we could deprecate with_scope across SDKs and then eventually remove it in some major.

[st0012]

Don't other SDKs have the same issue? What are their solutions to this?

[sl0thentr0py]

Yes the issue is the same across SDKs, so we can drive the change process by doing it in ruby first. But we cannot change the old with_scope, so it will have to be a new API.

[st0012]

That's fair. I'm not rushing to change it to be clear. Will adding a keyword argument to with_scope be an option? Like Sentry.with_scope(capture: true) { }

[sl0thentr0py]

hmm yea good point! As long as the old behavior is preserved/backward compatible, we should be good.

[krainboltgreene]

Wait, this isn't how with_scope is supposed to be used? Then how in the world is it supposed to be used? The docs are so absolutely bare bones of examples.

[benoittgt]

I am interested to work on this one and add the option to "capture". But I think it should be the default on a major version.