Skip to content

nginx and haproxy should respect Accept headers when responding with error templates #8179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
dianabarsan opened this issue Apr 14, 2023 · 4 comments · Fixed by #8446
Closed

nginx and haproxy should respect Accept headers when responding with error templates #8179

dianabarsan opened this issue Apr 14, 2023 · 4 comments · Fixed by #8446
Assignees
@nydr @dianabarsan
Labels
Type: Improvement Make something better
Milestone
4.4.0

Comments

@dianabarsan
Copy link
Member

dianabarsan commented Apr 14, 2023 •
edited
Loading

What feature do you want to improve?
Both nginx and haproxy use the default error templates for 4xx and 5xx responses. These templates are html.
Respecting the Accept header would standardize the behavior of the CHT, and this is important since we expect third party software to call CHT APIs.

To reproduce:

  • start CHT 4.x
  • stop your api container docker stop <api container name>
  • request a JSON response from API:
curl -k https://localhost -H 'Accept: application/json'
<html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx/1.19.6</center>
</body>
</html>

If you are logged into webapp, you will see a JSON parse error spammed in the console:
image

Describe the improvement you'd like
We should change nginx and haproxy configs to respond with JSON when the Accept: application/json header is passed.

Describe alternatives you've considered
Leave it as it is.

Additional context
I had a script that passed the result of an API call to jq. Because the response was html, my jq call failed.
Similarly, we've seen reports of feedback docs being generated that feedback docs get generated in webapp when with Unexpected token < in JSON at position 0:

#6412
#7782
@dianabarsan dianabarsan added the Type: Improvement Make something better label Apr 14, 2023
@dianabarsan dianabarsan self-assigned this Apr 14, 2023
@garethbowen
Copy link
Contributor

Similarly, we've seen

The suspense is killing me!

@dianabarsan
Copy link
Member Author

The suspense is killing me!

Apologies. Resuscitation in progress.

@nydr nydr self-assigned this Jul 26, 2023
@nydr nydr mentioned this issue Aug 10, 2023
Merged
3 tasks
@mrjones-plip
Copy link
Contributor

Also noting that the lack of a proper JSON and sending HTML instead affects more tickets than the two above. Spot checking one, I found: "This indicates that CouhchDb is down and Haproxy responds with html instead of JSON" #7759.

A fix to this will be a welcome addition!

@nydr
Copy link
Contributor

nydr commented Aug 17, 2023

We have suggested that HAProxy error pages to be JSON only since HAProxy is not externally available

@nydr nydr mentioned this issue Aug 22, 2023
Merged
2 tasks
nydr added a commit that referenced this issue Aug 24, 2023
@nydr
chore(#8179) Add json error messages to haproxy (#8481)
6c1b854
@garethbowen garethbowen mentioned this issue Aug 24, 2023
Closed
@mrjones-plip mrjones-plip mentioned this issue Aug 30, 2023
Closed
nydr added a commit that referenced this issue Aug 31, 2023
@nydr
fix(#8179) Add support for nginx and haproxy returning json error pag…
0086150 
…es (#8446)
@garethbowen garethbowen added this to the 4.4.0 milestone Aug 31, 2023
@mrjones-plip mrjones-plip mentioned this issue Sep 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nydr nydr

@dianabarsan dianabarsan

Labels
Type: Improvement Make something better
Projects
None yet
Milestone
4.4.0
Development

Successfully merging a pull request may close this issue.

fix(#8179) Add support for nginx and haproxy returning json error pages medic/cht-core
4 participants
@garethbowen @mrjones-plip @nydr @dianabarsan