Skip to content
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

Uploading debug information files fails in v2.33.1 #860

Closed
1 of 8 tasks
p0358 opened this issue Aug 3, 2024 · 9 comments · Fixed by #861 or #864
Closed
1 of 8 tasks

Uploading debug information files fails in v2.33.1 #860

p0358 opened this issue Aug 3, 2024 · 9 comments · Fixed by #861 or #864
Assignees

Comments

@p0358
Copy link

p0358 commented Aug 3, 2024

CLI Version

2.33.1

Operating System and Architecture

  • macOS (arm64)
  • macOS (x86_64)
  • Linux (i686)
  • Linux (x86_64)
  • Linux (armv7)
  • Linux (aarch64)
  • Windows (i686)
  • Windows (x86_64)

Operating System Version

Windows Server

Link to reproduction repository

https://github.com/p0358/black_market_edition/actions/runs/10231364720/job/28307610591

CLI Command

sentry-cli upload-dif --include-sources $files_to_upload

Exact Reproduction Steps

Compile a C++ app and:

          $files_to_upload = Get-ChildItem -Path build\bin -Recurse -Include ("*.pdb", "*.dll", "*.exe")
          echo "Files to upload:" $files_to_upload
          sentry-cli upload-dif --include-sources $files_to_upload

Expected Results

It should work like it did before. In v2.33.1 with PR getsentry/sentry-cli#2109 the issue getsentry/sentry-cli#2107 was reintroduced, which was previously fixed by getsentry/sentry-cli#2108, which had version v2.32.2. It looks like in both this repo and #816 somebody added a check for input files being UTF-8, seemingly forgetting that JavaScript is not the only language and that native apps with symbol files exist??? Barely any reasoning if at all was provided in the linked PRs as to what originally prompted such a check to be coded to begin with.

Proof for people with internal access at Sentry that it worked with symbolification and showing up source code and correct line of code: https://black-market-edition.sentry.io/issues/5651900111/?project=5545628

Actual Results

It fails:

Run $files_to_upload = Get-ChildItem -Path build\bin -Recurse -Include ("*.pdb", "*.dll", "*.exe")
Files to upload:
    Directory: D:\a\black_market_edition\black_market_edition\build\bin\x64-Staging
Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a---            8/3/2024 11:08 PM        1876480 bme.dll
-a---            8/3/2024 11:08 PM       20779008 bme.pdb
-a---            8/3/2024 11:07 PM         389120 curl.pdb
-a---            8/3/2024 11:07 PM         946176 discord-rpc.pdb
-a---            8/3/2024 11:07 PM         921600 imgui.pdb
-a---            8/3/2024 11:07 PM         183808 launcher.dll
-a---            8/3/2024 11:07 PM        4837376 launcher.pdb
-a---            8/3/2024 11:07 PM         110592 minhook.pdb
-a---            8/3/2024 11:07 PM         176128 nghttp2.pdb
-a---            8/3/2024 11:07 PM        3534848 spdlog.pdb
-a---            8/3/2024 11:07 PM         547328 Titanfall_alt.exe
-a---            8/3/2024 11:07 PM        4419584 Titanfall_alt.pdb
> Found 6 debug information files
error: file could not be read as UTF-8
  caused by: stream did not contain valid UTF-8
Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.
Error: Process completed with exit code 1.

Logs

There wouldn't be any extra info in debug log according to what I saw in other issues

@p0358 p0358 changed the title Uploading release files fails in v2.33.1 Uploading debug information files fails in v2.33.1 Aug 3, 2024
@szokeasaurusrex
Copy link
Member

Hi @p0358, thank you for reaching out and sorry for the inconvenience! We will investigate this as soon as possible – in the meantime, please work around the issue by downgrading to 2.32.2, or another version where the uploads still work

@szokeasaurusrex
Copy link
Member

There wouldn't be any extra info in debug log according to what I saw in other issues

@p0358 I am not sure where you saw this, I would expect we might see more logs.

Would you be able to provide the logs generated with --log-level=debug?

@szokeasaurusrex
Copy link
Member

Hi @p0358, I have been trying to reproduce this issue, but was unable to reproduce it on Mac. I tried with a Rust app as well as a C app.

It looks like the function modified in getsentry/sentry-cli#2109 is not actually called in code path used to upload debug information files via the debug-files upload command (the upload-dif command you are using is a soft-deprecated alias for debug-files upload). I confirmed this via a debugger, but it also makes sense because if the code path modified in getsentry/sentry-cli#2109 was hit, we would expect the CLI to simply skip over the non-UTF-8 file and log a message. Since you are observing a complete crash of the Sentry CLI, the error must be originating from elsewhere.

Could you please tell me which version of Sentry CLI is the most recent version that worked as expected?

@barnabwhy
Copy link

downgraded to v2.32.1 and this issue went away for me

@szokeasaurusrex
Copy link
Member

downgraded to v2.32.1 and this issue went away for me

That is good to know @barnabwhy.

Are you observing the issue on v2.32.2 and/or v2.33.0?

@barnabwhy
Copy link

downgraded to v2.32.1 and this issue went away for me

That is good to know @barnabwhy.

Are you observing the issue on v2.32.2 and/or v2.33.0?

I was experiencing this in latest version (v2.33.1(?)) but since i needed a quick fix i skipped straight to v2.32.1 since the other issue mentioning this first saw the problem in v2.32.2.
Sorry this doesn't give much extra info 😔

@p0358
Copy link
Author

p0358 commented Aug 8, 2024

This is a run on latest with debug log: https://github.com/p0358/black_market_edition/actions/runs/10309751662/job/28540026864

I have a suspicion now that maybe --include-sources causes this to happen, since I don't seem to be able to reproduce it locally by just uploading an arbitrary dll or pdb either. Sadly it doesn't seem like even the debug logging makes it easy to identify what file could have possibly directly caused the error, unless I'm blind...

@szokeasaurusrex
Copy link
Member

Indeed @p0358, looks like --include-sources could be the cause – if one of your debug files references as a source a non-UTF-8 file, then we would hit the error introduced in #816, and the error would not be handled.

I think we will need to fix this in symbolic. In the write_object_with_filter function (likely along the code path you are hitting), we need to skip over non-UTF-8 files here:

self.add_file(bundle_path, source, info)?;

@szokeasaurusrex
Copy link
Member

Just noticed this is actually not fully resolved, gonna open a PR in a sec with a fix.

szokeasaurusrex added a commit that referenced this issue Aug 30, 2024
#861 missed another spot where the ReadFailed error can cause the write function to fail; this commit fixes that.

Fixes #860
szokeasaurusrex added a commit that referenced this issue Sep 4, 2024
* feat(sourcebundle): Add callback to handle skipped files

Add a callback to SourceBundleWriter that is called every time we skip adding a file to the bundle due to a ReadFailed error.

Closes #863

* fix(sourcebundle): Skip all invalid sources

#861 missed another spot where the ReadFailed error can cause the write function to fail; this commit fixes that.

Fixes #860

* meta: Update changelog

* apply suggestions from code review

Co-authored-by: Jan Michael Auer <[email protected]>

---------

Co-authored-by: Jan Michael Auer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
5 participants