Skip to content

Supply event hint to async callback when possible#1189

Merged
st0012 merged 4 commits intomasterfrom
fix-async
Jan 8, 2021
Merged

Supply event hint to async callback when possible#1189
st0012 merged 4 commits intomasterfrom
fix-async

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Jan 8, 2021

This fixes #1188.

@st0012 st0012 added this to the 4.1.3 milestone Jan 8, 2021
@st0012 st0012 self-assigned this Jan 8, 2021
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 8, 2021 •
edited
Loading

Codecov Report

Merging #1189 (7d229d1) into master (6c0fe96) will increase coverage by 0.58%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1189      +/-   ##
==========================================
+ Coverage   97.95%   98.54%   +0.58%     
==========================================
  Files         191       96      -95     
  Lines        8058     4249    -3809     
==========================================
- Hits         7893     4187    -3706     
+ Misses        165       62     -103
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/client.rb 96.15% <100.00%> (+0.23%) ⬆️
sentry-ruby/spec/sentry/client_spec.rb 96.44% <100.00%> (+0.17%) ⬆️
sentry-raven/spec/raven/client_state_spec.rb
sentry-raven/spec/raven/utils/real_ip_spec.rb
sentry-raven/spec/raven/integrations/rake_spec.rb
...ven/spec/raven/processors/removestacktrace_spec.rb
sentry-raven/lib/raven/utils/request_id.rb
sentry-raven/lib/raven/integrations/rack.rb
sentry-raven/lib/raven/interfaces/exception.rb
sentry-raven/lib/raven/processor/sanitizedata.rb
... and 87 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c0fe96...7d229d1. Read the comment docs.

benoittgt
benoittgt reviewed Jan 8, 2021
View reviewed changes
sentry-ruby/spec/sentry/client_spec.rb
context "with 2 arity block" do
let(:async_block) do
lambda do |event, hint|
event["tags"]["hint"] = hint
Copy link
Copy Markdown

@benoittgt benoittgt Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that something that should maybe be documented. We jump from event object to and hash in async scenario with for example event.server_name to event['server_name'] for example.

I had the surprise yesterday. Maybe add a small notice when speaking about async in README.

Also the second args should be documented? Maybe you planned to do it in another PR 😊

Copy link
Copy Markdown
Contributor Author

@st0012 st0012 Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that something that should maybe be documented.

yeah I agree with this. thanks for reminding 😄

Also the second args should be documented?

if we consider "not passing event hint" as a bug, this change just fixes it to match the users' expectation. so I don't think it's something that worth document for. unless you're talking about updating the examples? then yes, I'll update them as well.

Copy link
Copy Markdown

@benoittgt benoittgt Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. More updating the example :)

@st0012 st0012 merged commit fb1a91b into master Jan 8, 2021
@st0012 st0012 deleted the fix-async branch January 8, 2021 10:15
@benoittgt
Copy link
Copy Markdown

benoittgt commented Jan 8, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@benoittgt benoittgt benoittgt left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@st0012 st0012

Labels

sentry-ruby

Projects

None yet

Milestone

4.1.3

Development

Successfully merging this pull request may close these issues.

Support event hint in async

3 participants

@st0012 @codecov-io @benoittgt