Skip to content

Conversation

@otaviolima
Copy link

@otaviolima otaviolima commented Dec 8, 2025

Only move the downloaded file if the HTTP status code is within the OK range

Motivation:

Our Package.swift has package dependencies that are hosted in our internal JFrog instance. We have noticed that when a server error occurs, a JSON payload is still returned. In our particular case the JFrog instance was returning HTTP 500 as the curl snippet below shows.

% curl --verbose -L \
  --netrc \
  --netrc-file bogusnetrc \
  -o "./Artifact.zip" \
  "https://host_redacted/Artifact.zip"
  <redacted logs>
 > GET /Artifact.zip HTTP/2
> Host: redacted_host
> Authorization: Basic <redacted>
> User-Agent: curl/8.7.1
> Accept: */*
> 
* Request completely sent off
< HTTP/2 500 
< date: Mon, 08 Dec 2025 11:01:50 GMT
< content-type: application/json;charset=ISO-8859-1
< content-length: 65
< 
% cat Artifact.zip 
{
  "errors" : [ {
    "status" : 500,
    "message" : ""
  } ]
}

Because of that, the error shown in the swift package PackagePath resolve is not very clear.

% swift package --package-path ./path/to/package resolve
Downloading binary artifact https://host_redacted/Artifact.zip
error: failed downloading 'https://host_redacted/Artifact.zip' which is required by binary target 'TargetName': $HOME_REDACTED/Library/Caches/org.swift.swiftpm/artifacts/ArtifactPath.zip already exists in file system
error: fatalError

This does not tell much about the exact underlying error that happened here.

Result:

After the changes in this PR, the error message will become much clearer about the underlying issue with the HTTP call

% ./build-folder/swift-package --package-path ./path/to/package resolve
Downloading binary artifact https://host_redacted/Artifact.zip
error: failed downloading 'https://host_redacted/Artifact.zip' which is required by binary target 'TargetName': badResponseStatusCode(500)
error: fatalError

response.statusCode < 200 || response.statusCode >= 300 {
// Delete the temporary file for bad response codes
try task.fileSystem.removeFileTree(path)
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of returning, perhaps this should throw so that the error handler below is consistently invoked for all types of failure?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @jakepetroules. I updated the PR following them.

Another change I made was to remove the lines that deleted the temporary file, based on the fact that the comment below states that the temporary file is cleaned up after delegate execution.

I tested again with my Package.swift and JFROG instance, and the error message is still the same one that I put in after changes in PR description, so from this side, the new code is yielding the same results as the initial implementation.

@otaviolima otaviolima force-pushed the fix-only-copy-files-httpOK branch from 9acd1ba to 1bb1d5a Compare December 9, 2025 03:03
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