Skip to content
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

calendar: including a large image will cause event submission to fail with server error #12

Closed
fool opened this issue Jul 14, 2018 · 9 comments
Closed

calendar: including a large image will cause event submission to fail with server error #12

fool opened this issue Jul 14, 2018 · 9 comments
Labels
backend bug high-priority

Comments

@fool
Copy link
Member

fool commented Jul 14, 2018

Reproduction steps:

  1. create an event, leave all defaults
  2. add required fields (event name, details, select a date, venue, address, check "I've read the comic box,
  3. add the attached image
  4. submit event (see popup and hit ok)
  5. notice no email about event being created & no event created

ccc logo

cc @sdobz
@fool fool added the bug label Jul 14, 2018
@sdobz
Copy link
Collaborator

sdobz commented Feb 16, 2019

Barring the other problems on the site it appears to work
The issue was a misconfiguration with the path that images stored in. See shift.overrides.production

@sdobz
Copy link
Collaborator

sdobz commented Mar 1, 2019

WORKS NOW

@sdobz sdobz closed this as completed Mar 1, 2019
@fool
Copy link
Member Author

fool commented Apr 20, 2019

This does not in fact work perfectly, reopening. These two images don't work:

jpg:

image

png: (awaiting from bug-reporter)

@fool fool reopened this Apr 20, 2019
@fool fool changed the title including a PNG will cause event submission to fail silently including a large image will cause event submission to fail silently Apr 20, 2019
@fool fool changed the title including a large image will cause event submission to fail silently including a large image will cause event submission to fail with server error Apr 20, 2019
@fool fool changed the title including a large image will cause event submission to fail with server error calendar: including a large image will cause event submission to fail with server error Oct 22, 2019
@fool fool mentioned this issue Oct 22, 2019
Closed
@carrythebanner carrythebanner mentioned this issue Feb 5, 2020
Merged
@fool
Copy link
Member Author

fool commented May 18, 2022

tried to debug a bit but didn't quite get to the end.

Here's a decent view that I (as a netlify employee) can see of failures from our log service, so we can use them to find entries in the local logs to dig into further. not all the failures are HTTP 413, but the actionable ones sure all are!

https://cloud.us.humio.com/netlify-us-production/search?query=domain%3D%2A.shift2bikes.org%7Cmethod%3DPOST%7Cstatus%3D413&live=false&start=7d

@fool
Copy link
Member Author

fool commented May 18, 2022

Once we fix #426 this will become easier to debug...

@ionous
Copy link
Contributor

ionous commented May 12, 2023

i suspect that the fix for #483 fixes this.
manage_event.php has a 2MB limit set ( the above image is 9.3 MB )
previously if the validator failed, the code would just keep going, resulting in a server error rather than a valid response with an error json string. now it should be returning the proper error string.
not 100% sure what the client will see, but i can try it later and see...

https://github.com/shift-org/shift-docs/blob/main/backend/www/manage_event.php#L80

@ionous
Copy link
Contributor

ionous commented May 16, 2023

so it turns out, we never hit the manage_event.php limit because nginx has a 1meg limit.

to verify that, i made a 1.5 meg version of the problematic image, and couldn't upload it ( running local docker )
then i changed shift.conf to use client_max_body_size 2m; and was able to upload it.

we should amend the config to match the php, but the client will also need adjustment to handle the error.
( or, i suppose we could disable the ngnix limit but it seems like a nice protection )

http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size

nginx returns 413 for the endpoint, with an html response; the client is expecting a json response and no error(s).

<head><title>413 Request Entity Too Large</title></head>
<body>
<center><h1>413 Request Entity Too Large</h1></center>
<hr><center>nginx/1.23.1</center>
</body>
</html>

shift-1_5

@ionous ionous mentioned this issue May 16, 2023
Merged
ionous added a commit to ionous/shift-docs that referenced this issue Jun 14, 2023
@ionous
on the client, handle the two types of image too large errors ( 400 a…
fb4643e 
…nd 413 )

so that they act like the errors of other fields.
1. check for the nginx 413 error, and create a fake error message in that case
2. swap the "file" field error for "image" so that it shows on the appropriate html label

shift-org#12
@ionous ionous mentioned this issue Jun 14, 2023
Merged
@ionous
Copy link
Contributor

ionous commented Jun 14, 2023

i've created a pr to handle this a bit better on the client side; here's what it will look like now:

image

@ionous
Copy link
Contributor

ionous commented Jun 15, 2023

fixed as of #523

@ionous ionous closed this as completed Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend bug high-priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fool @ionous @sdobz