GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Handle curl exit status 56 as a protocol failure in installer/updater. by Ferroin · Pull Request #24276 · netdata/netdata

Conversation

Ferroin commented Oct 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Summary

Apparently some versions of curl may return exit status 56 on certain protocol errors, so we need to handle that just like exit statuses 22 and 78.

Test Plan

n/a

Additional Information

Fixes: #24046

Summary by cubic

Treats curl exit status 56 as a protocol failure in the installer and updater. Some curl versions return status 56 on certain protocol errors, so it is now handled like statuses 22 and 78, removing the partial download and reporting the failure with a clear warning.

Fixes #24046.

^{Written for commit e54c47b. Summary will update on new commits.}

Summary by CodeRabbit

Bug Fixes
- Improved handling of interrupted downloads: partial files are removed, and results are reported according to the received HTTP status.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Handle curl exit status 56 as a protocol failure in installer/updater.

e3c71f6

Apparently some versions of curl may return exit status 56 on certain
protocol errors, so we need to handle that just like exit statuses 22 and
78.

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 222f545f-52bf-4caa-8ffa-1bdeaa452cc9

📥 Commits

Reviewing files that changed from the base of the PR and between dff79a1 and e54c47b.

📒 Files selected for processing: 2

packaging/installer/kickstart.sh
packaging/installer/netdata-updater.sh

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The installer download handlers now treat curl exit code 56 as an HTTP-status-related result. Kickstart removes partial downloads and handles logged 2xx statuses as protocol errors. The updater routes code 56 through its existing HTTP-status mapping.

Changes: Download error handling

Layer / File(s)	Summary
Curl 56 HTTP-status handling `packaging/installer/kickstart.sh`, `packaging/installer/netdata-updater.sh`	Kickstart removes partial downloads and handles logged 2xx statuses as protocol errors. The updater removes the destination when applicable and maps the logged status through its existing error handling.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to e54c4

The .tar.gz fallback remains available after a failed .tar.zst download, so no actionable merge blocker is established.

### Architecture Summary

Architecture risk: 🔵 Low · up to e54c4

The change affects 1 system.

Changed systems: packaging

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — packaging (service) was modified; 2 changed files map to changed impact.

Before / after behavior

observed — Modified behavior in packaging/installer/kickstart.sh: handle_curl_result adds handling for curl result 56: it removes any partial destination, returns 4 with a protocol-error warning for logged statuses beginning with 2, and otherwise delegates status handling to handle_http_status.
observed — Modified behavior in packaging/installer/netdata-updater.sh: Curl exit code 56 now shares the HTTP-status handling path with 22 and 78; previously, it fell through to the generic error case.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		The change addresses the main `#24046` failure. `kickstart.sh` now handles curl status 56, removes the partial destination, and routes the logged HTTP status through the existing HTTP handler. `netdata-…	Change `deferred_warnings()` to use a fixed `printf` format and pass `NETDATA_WARNINGS` as data. Verify that warnings containing `%{http_code}` print without a format error.
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: handling curl exit status 56 as a protocol failure in the installer and updater.
Out of Scope Changes check		The changes are limited to curl failure handling in the installer and updater scripts. Both changes support the curl status 56 behavior described by `#24046` and the pull request. No unrelated file or b…

Full details: Out of Scope Changes check

Explanation

The changes are limited to curl failure handling in the installer and updater scripts. Both changes support the curl status 56 behavior described by #24046 and the pull request. No unrelated file or behavior change is present in the whole-pull-request diff.

Full details: Linked Issues check

Explanation

The change addresses the main #24046 failure. kickstart.sh now handles curl status 56, removes the partial destination, and routes the logged HTTP status through the existing HTTP handler. netdata-updater.sh applies the same status mapping. However, #24046 also reports the deferred_warnings() format-string warning. This pull request does not change that warning path, so %{http_code} can still be interpreted by printf.

Resolution

Change deferred_warnings() to use a fixed printf format and pass NETDATA_WARNINGS as data. Verify that warnings containing %{http_code} print without a format error.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

CI annotation context is incomplete: annotation collection unavailable or limited

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot added the area/packaging Packaging and operating systems support label

Oct 12, 2026

cubic-dev-ai Bot reviewed

Oct 12, 2026

View reviewed changes

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

All reported issues were addressed across 2 files

Architecture diagram

sequenceDiagram
    participant KS as kickstart.sh
    participant NS as netdata-updater.sh
    participant CURL as curl
    participant LOG as Download Log
    participant FS as Filesystem

    Note over KS,NS: Installer/Updater Download Protocol Failure Handling

    KS->>CURL: Download file (with log)
    CURL-->>KS: Exit status 56 (protocol error)

    KS->>KS: handle_curl_result - evaluate exit code
    alt Exit status 56 (protocol failure)
        KS->>LOG: Read last line for HTTP status
        LOG-->>KS: HTTP status message
        KS->>FS: Remove partial download
        KS->>KS: handle_http_status - report failure
    end

    NS->>CURL: Download file (with log)
    CURL-->>NS: Exit status 56 (protocol error)

    NS->>NS: _safe_download - evaluate result
    alt Exit status 56 (protocol failure)
        NS->>FS: Remove partial download (unless /dev/null)
        NS->>LOG: Read last line for HTTP status
        LOG-->>NS: Status code (e.g., 404)
        alt Status 404
            NS-->>NS: Return 1 (not found)
        else Other status
            NS-->>NS: Return 2 (generic failure)
        end
    end

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

packaging/installer/kickstart.sh Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Ensure proper error message for exit status 56.

e54c47b

Ferroin requested a balanced review from Copilot

October 12, 2026 12:39

Copilot started reviewing on behalf of Ferroin

October 12, 2026 12:40

View session

Copilot AI reviewed

Oct 12, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🟡 Changes recommended

Exit 56 can produce misleading diagnostics for non-HTTP log values, and the regression path lacks automated coverage.

1 open finding

Add regression tests for exit 56 HTTP and protocol failures · New

What changed in this PR

Handles curl exit status 56 as a recoverable protocol/download failure.

Changes:

Classifies curl exit 56 in installer and updater download handling.
Removes partial downloads and preserves archive fallback behavior.

File	Description
`packaging/installer/netdata-updater.sh`	Handles exit 56 during safe downloads.
`packaging/installer/kickstart.sh`	Adds protocol-error handling for exit 56.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

packaging/installer/kickstart.sh Resolved

Uh oh!

There was an error while loading. Please reload this page.

Ferroin commented Oct 19, 2026

Copy link

Copy Markdown

Member Author

Closing and re-opening to try and force CLA assistant to re-check the PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ferroin closed this

Oct 19, 2026

Ferroin reopened this

Oct 19, 2026

sonarqubecloud Bot commented Oct 19, 2026

Copy link

Copy Markdown

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ferroin marked this pull request as ready for review

October 19, 2026 13:55

Ferroin requested a review from a team as a code owner

October 19, 2026 13:55

Ferroin requested a review from a team

October 19, 2026 14:00

thiagoftsm approved these changes

Oct 19, 2026

View reviewed changes

thiagoftsm left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Testing on Debian 13, no issue was found. LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ferroin merged commit 0f623cf into netdata:master

Oct 20, 2026

319 of 320 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Ferroin deleted the dist-gzip-fallback-fix branch

October 20, 2026 07:05

stelfrag mentioned this pull request

Oct 26, 2026

Patch release 2.12.3 #24442

Merged

Ferroin added a commit that referenced this pull request

Nov 4, 2026


          Handle curl exit status 56 as a protocol failure in installer/updater. (

271f2cc

#24276)

* Handle curl exit status 56 as a protocol failure in installer/updater.

Apparently some versions of curl may return exit status 56 on certain
protocol errors, so we need to handle that just like exit statuses 22 and
78.

* Ensure proper error message for exit status 56.

(cherry picked from commit 0f623cf)

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

cubic-dev-ai[bot]

cubic-dev-ai[bot] left review comments

thiagoftsm

thiagoftsm approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

area/packaging

Packaging and operating systems support

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: kickstart.sh local build aborts instead of falling back to .tar.gz when .tar.zst is missing (curl exit 56 unhandled)

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Ferroin commented Oct 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Additional Information

Summary by cubic

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Ferroin commented Oct 19, 2026

Uh oh!

sonarqubecloud Bot commented Oct 19, 2026

Quality Gate passed

Uh oh!

thiagoftsm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ferroin commented Oct 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.