sentry-dart icon indicating copy to clipboard operation
sentry-dart copied to clipboard

fix: Scope.clone incorrectly accesses tags

Open quaaantumdev opened this issue 3 years ago • 1 comments

:scroll: Description

fixed Scope.clone, incorrectly accesses tags and therefore crashes. Main branch currently extracts the keys from _tags to iterate over. This is all fine but it the tries to access the values via the keys-list which makes no sense, it's a list of strings, not a map where you could actually use the keys. I guess it's simply a typo due to "not so optimal" naming.

:bulb: Motivation and Context

Sentry crashing, error reporting with tags does not work. The following code will crash (current main branch):

Sentry.configureScope((scope) async {
  await scope.setTag('some.tag', 'value');
});

await Sentry.captureException(
  error,
  stackTrace: stackTrace,
  hint: 'here goes an error',
  withScope: (scope) async {
    try {
      await scope.setTag('class', className);
    } catch (err, stack) {
      print('ERROR: failed to add class in sentry: $err\n$stack');
    }
  },
);

:green_heart: How did you test it?

running in production just fine :) currently using it via a pubspec.yaml git reference:

 sentry: # also remove override when fixed
    git:
      url: https://github.com/quaaantumdev/sentry-dart.git
      path: dart

would like to upgrade back to the official version once fixed

:pencil: Checklist

  • [x] I reviewed submitted code
  • [ ] I added tests to verify changes
  • [ ] I updated the docs if needed
  • [ ] All tests passing
  • [x] No breaking changes

:crystal_ball: Next steps

let's make sentry great again ;-)

#skip-changelog

quaaantumdev avatar Aug 06 '22 15:08 quaaantumdev

Codecov Report

Merging #978 (0cc8cbe) into main (1998016) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #978   +/-   ##
=======================================
  Coverage   91.14%   91.14%           
=======================================
  Files           9        9           
  Lines         192      192           
=======================================
  Hits          175      175           
  Misses         17       17           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 08 '22 06:08 codecov-commenter

@quaaantumdev thanks for the PR, can you explain which issue are you getting? what's the error? Please elaborate on:

incorrectly accesses tags and therefore crashes

marandaneto avatar Aug 17 '22 07:08 marandaneto

By incorrectly accesses tags and therefore crashes he means the variable name.

Old code:

final tags = List.from(_tags.keys); //tags is a list of string
     for (final tag in tags) {
       final value = tags[tag]; //tags is not a dictionary, we should be reading from _tags passing the key
       if (value != null) {
         clone._setTagSync(tag, value);
       }
    }

brustolin avatar Aug 17 '22 07:08 brustolin