Skip to content

Make Puma low-level handler return recommended status code

Stan Hu requested to merge sh-fix-puma-low-level-handler-6.4.0 into master

What does this MR do?

Because of https://github.com/puma/puma/pull/3094, Puma v6.4.0 now returns the status code sent from the low-level handler instead of Puma's recommended status code. This caused malformed URLs to return a 500 error when previously it would return a 400 Bad Request error, which skewed error metrics.

Previously we had hard-coded low-level errors to return a 500 error, but now we should adapt this to return the status code recommended by Puma.

Related issues

Relates to gitlab-com/gl-infra/production#16417 (closed)

Related merge request:

gitlab-org/gitlab#426135 (closed) has been created to address this duplication.

How to test locally

With the current puma.rb:

$ curl -I 'https://gitlab.example.com/-/merge_requests?sort=created_date&state=<th:t=\"%24{dfb}%23foreach'
HTTP/2 500
date: Mon, 25 Sep 2023 17:23:03 GMT
content-type: text/html; charset=utf-8
content-length: 3037
cache-control: no-cache, no-store, max-age=0, must-revalidate
expires: Fri, 01 Jan 1990 00:00:00 GMT
pragma: no-cache
strict-transport-security: max-age=63072000
referrer-policy: strict-origin-when-cross-origin

Install the Helm Chart with --set gitlab.webservice.image.tag=sh-fix-puma-low-level-handler-6-4-0. You should now see:

$ curl -I 'https://gitlab.example.com/-/merge_requests?sort=created_date&state=<th:t=\"%24{dfb}%23foreach'
HTTP/2 400
date: Mon, 25 Sep 2023 17:18:05 GMT
strict-transport-security: max-age=63072000
referrer-policy: strict-origin-when-cross-origin

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion

Required

  • Merge Request Title, and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com
  • When ready for review, MR is labeled "~workflow::ready for review" per the Distribution MR workflow

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Integration tests added to GitLab QA
  • The impact any change in container size has should be evaluated
  • New dependencies are managed with dependencies.io
Edited by Stan Hu

Merge request reports