[Bug]: artifact cache upload missing return after storage.Write error (double response + useCache)

Open Beginner friendly
#6,131 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
84/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
go
Domain
backend

Research direction

Start in pkg/artifactcache/handler.go at the upload/Write path and reproduce the failure by making the cache root unusable, then PATCH with Content-Range and a body. Ensure the storage.Write error response ends the handler before useCache or the success response. Add a regression test that verifies one 500 response and no success-side effects.

Written by the indexing model from the issue text.

Description

Describe the bug

In the artifact cache upload handler, when storage.Write fails the code responds with 500 but does not return, then continues as if the upload succeeded:

if err := h.storage.Write(cache.ID, start, r.Body); err != nil {
    h.responseJSON(w, r, 500, err)
}
h.useCache(id)
h.responseJSON(w, r, 200)

On Write failure this:

  1. Emits a 500 JSON error body
  2. Still calls h.useCache(id) (DB touch as if success)
  3. Calls responseJSON(200) again → http: superfluous response.WriteHeader and a second body fragment

Every other error branch in this handler returns after responseJSON.

Debug information
  • Checkout @ 4f411281417e88660bea1c1a1749aa71ae0bd60f
  • File: pkg/artifactcache/handler.go (upload / Write path)
Repro steps

Force storage.Write to fail (e.g. cache root is not a usable directory), then PATCH the cache upload endpoint with Content-Range + body.

Observed pattern:

  • status 500
  • response body like {"error":"..."}{}
  • log: superfluous response.WriteHeader from responseJSON
Expected behavior

On Write error: respond once with 500 and return; do not call useCache or emit 200.

Actual behavior

Double response + useCache after failure.

Suggested direction
if err := h.storage.Write(cache.ID, start, r.Body); err != nil {
    h.responseJSON(w, r, 500, err)
    return
}

I'd be happy to open a PR with a regression test.

Dominant language
Go
Stars
72.1k
Forks
2k
PR merge metrics
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from nektos/act

All issues in nektos/act

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.