getsentry / craft

The universal Sentry release CLI 🚀
MIT License
133 stars 15 forks source link

Extracted artifact contains broken zip files #518

Closed vaind closed 8 months ago

vaind commented 9 months ago

Environment

Steps to Reproduce

When trying to add powershell target, I'm consistently getting:

ℹ Extracting artifact "Sentry.zip"                                                                                                                   [target/powershell] 16:39:12  

 ERROR  invalid distance code                                                                                                                                            16:39:12  

  at Zlib.zlibOnError [as onerror] (node:zlib:189:17)

when extracting a zip file that came as part of the artifact zip from the following artifact

   {
      "id": 1278813393,
      "node_id": "MDg6QXJ0aWZhY3QxMjc4ODEzMzkz",
      "name": "fccdee8858a22a156012d96b8819c31f76a6c00d",
      "size_in_bytes": 1391448,
      "url": "https://api.github.com/repos/getsentry/sentry-powershell/actions/artifacts/1278813393",
      "archive_download_url": "https://api.github.com/repos/getsentry/sentry-powershell/actions/artifacts/1278813393/zip",
      "expired": false,
      "created_at": "2024-02-27T13:35:06Z",
      "updated_at": "2024-02-27T13:35:06Z",
      "expires_at": "2024-05-27T13:34:52Z",
      "workflow_run": {
        "id": 8065639639,
        "repository_id": 755140913,
        "head_repository_id": 755140913,
        "head_branch": "release/0.0.1",
        "head_sha": "fccdee8858a22a156012d96b8819c31f76a6c00d"
      }
    },

The first zip file (fccdee8858a22a156012d96b8819c31f76a6c00d.zip) that comes directly from GitHub is extracted without any errors reported but actually, its content that is written to disc is broken (Sentry.zip)

I'm going to check if the same happens when using yauzl to extract instead of unzipper

asottile-sentry commented 9 months ago

this looks similar to the errors we were seeing with newer node versions -- are you using the same docker image?

vaind commented 9 months ago

I'm running outside docker because the way the dockerfile is currently structured currently doesn't work with the development loop I'm working with. Also, it doesn't actually rebuild with yarn when you run docker build . due to some caching issue 🤷

anyway, the same issue was in node-unzip which is what unzipper is forked from. It's just a bug somewhere in that dependency and from the state of that project, doesn't seem worth pursuing. Instead, I've tried yauzl and it works flawlessly.

asottile-sentry commented 9 months ago

would you like to send a patch that improves the unpacking here? it would probably be nice to fix it so it supports whatever other zips are involved that it was unhappy with

vaind commented 9 months ago

yeah, I'm preparing a PR

vaind commented 8 months ago

I've found out it happens only with node20, as you've already suggested it might.

I've added a test case for the future update and leaving the actual implementation untouched for now.

vaind commented 8 months ago

I've found out it happens only with node20, as you've already suggested it might.

I couldn't reproduce this in a test case, seems to be limited to yarn build output.