-
-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sentry-cli debug-files check
doesn't seem to work for wasm
#848
Comments
Hey, thanks for reporting the issue. Do you have any concrete wasm file that we could use to debug this? https://github.com/getsentry/sentry-cli/releases We bumped |
@kamilogorek I can provide a wasm file, what's the best way to share it? It seems to be reproducible with any wasm file that has a build_id inserted by the wasm-split tool https://github.com/getsentry/symbolicator/tree/master/crates/wasm-split. |
I see you have some binaries in the test folder here https://github.com/getsentry/sentry-cli/tree/master/tests/integration/_fixtures, how about we do some test-driven development, I'll put up a PR with a wasm binary with debug symbols and a build_id, plus a test case with the wrong expected output, and then we'll go from there? Would be awesome to get some wasm tests in here so we avoid regressions |
Sounds great! |
And sure enough it seems to pass... getsentry/sentry-cli#1205, will have to dig in deeper |
I wonder if this is potentially a hint: For the trivial binary where this works (the test from the PR above), we get:
From a real-world binary I see:
Is there a difference between type |
The latter one is matched by this branch - https://github.com/getsentry/sentry-cli/blob/512d98349008584e7c27dcde1eacf402a1c67dd6/src/utils/dif.rs#L260-L263 which allows it to print not only I believe we still need the repro case for broken scenario to debug this. cc @Swatinem |
I have a repro case that I can't easily share as the debug symbols contain references to internal code. I will work to create a simpler one, but not sure how hard it will be to do so |
This issue 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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
still reproducible FYI |
@bkotzz Can you share the steps/file you used to reproduce this issue? |
Hey @szokeasaurusrex I've been trying to get a repro but so far I'm only able to reproduce it with a debug symbols wasm binary when it has a lot of debug information that I can't share externally. I basically have a 200MB debug symbols file, that I know has a build_id custom section:
Where the sentry CLI says no debug ID exists:
I narrowed the problem down to the "name" custom section. If I remove the name custom section:
Then check is able to find the build_id
So I am wondering if there is some code parsing the custom sections in the order they appear in the binary, and something gets tripped up when it hits the "name" section, and doesn't continue on the the build_id section? |
@bkotzz to clarify, is this issue blocking you from achieving something in your workflow? Does the problem only affect |
Hi @szokeasaurusrex, it's not just This is only a problem in recent versions of sentry CLI, like 2.31.0. Uploading works fine with 1.71.0, as the old CLI is able to find the build ID |
By "recent versions," do you mean all 2.x versions? What is the latest version that the command still works? |
yeah all V2 versions can repro the problem. 1.71.0 works |
Hi all, I also came across this problem and with your tips I went a step further. @bkotzz that was a very good lead with the 'name' section 💪 Long story short: The problem is when the Just handling those exception is not a solution, because we need a fully (1) It seems that easiest solution is just grow It's also possible that I'm a little wrong, due to my little experience. To get to what I wrote above, I switched between different committees in the sentry-cli repo.
because log was added LINK:
(2) Even though it looks like it works in version Ending this long message:
Nevertheless, I hope that I helped somehow and that this problem can be solved. PS: I also have an additional question: is this code also used on the cloud server side? I wonder if even if I fixed it locally (increasing limit size), would the symbols be recognized correctly on the sentry cloud side? |
Hey @trzeciak, thank you for the detailed investigation! Based on this information, it seems that the underlying problem is originating from Symbolic, specifically from one of Symbolic's dependencies ( I am going to transfer this issue over to the |
I'm spending extra time on this issue and it looks like the size of 100_000 is for a single function-symbols entry containing vector of all symbols used in function (not the entire name section as I previously thought) https://webassembly.github.io/spec/core/appendix/custom.html. I checked it and it seems that in my case I exceed this value by a few percent, so I proposed to slightly increase this limit for now, but ultimately I think that the implementation in wasmpaarser should be improved bytecodealliance/wasm-tools#1650 The workaround for this is probably to split a specific function that exceeds this limit, I'll try to check that. |
Unfortunately I wasn't able to find it. I tried to help myself with the tools: Next workaround is just use sentry cloud to aggregate and group crashes, and make demangling manualy on local PC 😢 |
Okay, after update
My However, I end up getting a different error when trying to upload the symbols, and the second time it claims it's on the server.
I think I'll report this as a separate bug, and this ticket can be closed once the aforementioned PR has been merged. |
Fixed by #853 and getsentry/sentry-cli#2120 |
Fix a wasm too big name section and legacy exceptions support. It should fix this issue: getsentry/symbolic#848 Related to getsentry/symbolic#853
Environment
How do you use Sentry?
Can repro with both
Which SDK and version?
Using sentry-cli v2.0.3
Steps to Reproduce
% sentry-cli debug-files check some.symbols.wasm
Expected Result
Something like:
This seems to be a regression as this used to work around v1.70.1
I can verify the build_id field is present in the wasm binary:
Actual Result
The text was updated successfully, but these errors were encountered: