Skip to content

Conversation

kriswest
Copy link
Contributor

resolves #1177
PR #1164 changed the status codes returned on rejected pushes to better represent the outcome (forbidden 403, invalid request 401, server error 500) however git clients don't render the message returned for these status codes. Hence, this PR switches all response back to the 200 status code, which will, render the message. It also adds a comment on each rejection to inform future maintainers why that code is returned.

When a 403 is returned on reject:

% git push
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 14 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 309 bytes | 309.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
error: RPC failed; HTTP 403 curl 22 The requested URL returned error: 403
send-pack: unexpected disconnect while reading sideband packet
fatal: the remote end hung up unexpectedly
Everything up-to-date

When a 200 is returned on reject:

% git push
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 14 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 309 bytes | 309.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
remote: 	Your push has been blocked ([email protected] is not allowed to push on repo https://github.com/*****.git)
fatal: the remote end hung up unexpectedly
fatal: the remote end hung up unexpectedly
error: failed to push some refs to 'http://localhost:8000/github.com/*****.git'

Copy link

netlify bot commented Aug 29, 2025

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 7210153
🔍 Latest deploy log https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/68b1ba6d7511050009a2a2b6

@kriswest kriswest requested a review from a team August 29, 2025 14:33
@github-actions github-actions bot added the fix label Aug 29, 2025
@kriswest
Copy link
Contributor Author

@jescalada one more to get in - I was testing the response directly and not whether they render through a git client nicely, which they did not.

Copy link

codecov bot commented Aug 29, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.76%. Comparing base (00c11bd) to head (7210153).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/proxy/routes/index.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1178   +/-   ##
=======================================
  Coverage   82.76%   82.76%           
=======================================
  Files          66       66           
  Lines        2780     2780           
  Branches      332      332           
=======================================
  Hits         2301     2301           
  Misses        431      431           
  Partials       48       48           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@jescalada jescalada left a comment

Choose a reason for hiding this comment

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

LGTM! I noticed this issue as well, thanks for diagnosing and fixing it so quickly! 🚀

@kriswest kriswest merged commit 2950617 into finos:main Aug 29, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pushes rejected with 403 status message don't render the message at the git client end
2 participants