Skip to content

fix: better error logging in gitter - #5892

Open
Ly-Joey wants to merge 3 commits into
gitter-less-resetfrom
gitter-better-err
Open

fix: better error logging in gitter#5892
Ly-Joey wants to merge 3 commits into
gitter-less-resetfrom
gitter-better-err

Conversation

@Ly-Joey

@Ly-Joey Ly-Joey commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Previously, gitter errors are logged at request level so when a singleflight group fails for whatever reason, all requests in that group will log the same error and make the logs unnecessarily noisy.
This PR moves the logging inside the singleflight groups' Do() so one group only emit one set of logs.

Partially fixes: #5805


Stack created with GitHub Stacks CLIGive Feedback 💬

@Ly-Joey
Ly-Joey force-pushed the gitter-better-err branch from ecca097 to 4d085fb Compare August 24, 2026 03:35
@Ly-Joey
Ly-Joey force-pushed the gitter-better-err branch 2 times, most recently from aca8f62 to b4a50de Compare August 24, 2026 03:52
@Ly-Joey
Ly-Joey force-pushed the gitter-better-err branch from b4a50de to dc1cd24 Compare August 24, 2026 04:08
@Ly-Joey
Ly-Joey requested review from a team, another-rex and cuixq and removed request for another-rex August 24, 2026 04:21
Comment thread go/cmd/gitter/errors.go
return http.StatusForbidden // 403
case isRateLimitError(err):
return http.StatusTooManyRequests // 429
case isRemoteHostError(err):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How do you feel about using 502 Bad Gateway (http.StatusBadGateway) instead of 424 Failed Dependency here?

  • 424 is a 4xx code (client error), which indicates that the caller made an invalid request or provided invalid state, but isRemoteHostError seems focus more on the upstream server errors, where the caller's request was valid.
  • gitter acts as an intermediary/proxy to upstream remote git hosts, 502 Bad Gateway feels like the more suitable status code for upstream server failures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point! Updated :D

@Ly-Joey
Ly-Joey force-pushed the gitter-better-err branch from 7c302ac to f243752 Compare August 24, 2026 05:41
cuixq
cuixq previously approved these changes Aug 24, 2026
@Ly-Joey
Ly-Joey force-pushed the gitter-better-err branch from 4ab539e to 7682792 Compare August 25, 2026 01:18
@Ly-Joey
Ly-Joey requested a review from cuixq August 25, 2026 01:20
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.

Reduce Gitter Logging Noise

2 participants