Skip to content

Don't mask real error when converting an event to JSON fails#1797

Closed
drcapulet wants to merge 1 commit intogetsentry:masterfrom
drcapulet:alexc-async-crash
Closed

Don't mask real error when converting an event to JSON fails#1797
drcapulet wants to merge 1 commit intogetsentry:masterfrom
drcapulet:alexc-async-crash

Conversation

@drcapulet
Copy link
Copy Markdown
Contributor

If the event is unable to be converted to JSON, trying to key into event_hash errors because it is nil. I'm going to look into how to prevent that issue in a later PR.

@st0012
Copy link
Copy Markdown
Contributor

st0012 commented Apr 26, 2022

It looks like such error only happens at the line of event_hash = event.to_json_compatible? And thus cause event_hash to be nil.

If that's the case, perhaps we should rescue that method call to make the case explicit? Something like:

begin
  event_hash = event.to_json_compatible
rescue => e
  log_error("Converting #{event.inspect} to JSON compatible hash failed", e, debug: configuration.debug)
end

@github-actions
Copy link
Copy Markdown

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@st0012
Copy link
Copy Markdown
Contributor

st0012 commented Jul 18, 2022

@drcapulet Do you want to continue on this one? Or you're ok with me opening another PR to finish it?

@st0012
Copy link
Copy Markdown
Contributor

st0012 commented Jul 23, 2022

Closing in favor of #1853

@st0012 st0012 closed this Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants