augustin-wien / augustina-backend

An open-source web shop designed for selling magazines on the street.
GNU Affero General Public License v3.0
7 stars 0 forks source link

Fix image upload settings #135

Closed jofmi closed 10 months ago

jofmi commented 11 months ago

Type of change

Description

Fixed WriteJSON payload for updateSettings.

The wrong payload to WriteJSON broke the container without throwing an error - this issue is still not solved as I do not know the cause.

Checklist:

jofmi commented 11 months ago

Hi @lebe1, both errors do not appear in my local setup.

lebe1 commented 11 months ago

Using your frontend branch fix-settings-update,I have two issues, I would like you to reproduce.

  1. Uploading an image with a long weird name like Screenshot from 2023-12-01 10-29-03.png leads to the error "GET http://localhost:3000/[object%20File] HTTP/1.1"
  2. Updating to the main item works fine with newspaper and calendar but item "Digitale Zeitung" leads to the error state that no database values are shown in the backoffice. I think this a frontend error of a hardcoded value, which I thought you solved @nanu-c but maybe not in the settingsupdate frontend page?

After a more extended testing I found more details on these issues.

1. Error of uploading images This error only occurs in my Chrome browser not in my Firefox browser. Therefore, I assume this is a Frontend error. The "GET http://localhost:3000/[object%20File] HTTP/1.1" call is created after selecting an image to be uploaded. @nanu-c do you have any ideas?

2. Error of changing main items several items This error seems to me now as a backend error. In my Chrome browser as well as in my Firefox browser i can reproduce this error after several updating the main item between newspaper, calendar and digital license. Therefore, something still muss be broken in the backend, when not uploading an image during a settings update. @jofmi can you try reproducing this again?

jofmi commented 11 months ago

Hi @lebe1 I've been trying to change images and item back and forth and still cannot reproduce the error :/

lebe1 commented 11 months ago

Hi @jofmi and @nanu-c, this issue is really giving me headaches. I would like to but really having a hard time simply ignoring this state of the system where the database is not connected to the backend and not throwing any errors.\ In the following two videos I could reproduce both problems on Firefox:

Screencast_Firefox_DB_not_connected.webm

Screencast_Firefox_file_path.webm

Here are both errors shown on Chrome:

Screencast_Chrome_DB_not_connected.webm

Screencast_Chrome_file_path.webm

nanu-c commented 11 months ago

1. Error of uploading images This error only occurs in my Chrome browser not in my Firefox browser. Therefore, I assume this is a Frontend error. The "GET http://localhost:3000/[object%20File] HTTP/1.1" call is created after selecting an image to be uploaded. @nanu-c do you have any ideas?

This part is now solved here -> https://github.com/augustin-wien/augustina-frontend/pull/92

nanu-c commented 11 months ago

And also i can't reproduce the main item change all breaks error either here.

jofmi commented 11 months ago

@lebe1 i have tried reproducing again with your video, with no success. i also tried to create a new item like it was already available in your video, which also created no issues. Can you see if the error still appears on your end if you use Aarons new branch https://github.com/augustin-wien/augustina-frontend/pull/92? I also upgraded pgx to the latest version to see if this can fix your issue. Best i can suggest otherwise is that I take a look at the issue on your laptop directly.

jofmi commented 11 months ago

I have the thought that this might be caused by the frontend sending two overlapping db requests - since it happened when the frontend had the loop issue of sending GET and PATCH requests to the backend over and over again.

Maybe connected: https://github.com/jackc/pgx/issues/635, esp. https://github.com/jackc/pgx/issues/635#issuecomment-686721489

@nanu-c, might there be a fundamental mistake how we do queries?

We could add some healthcheck config to pgxpool.New in db.go:120 https://pkg.go.dev/github.com/jackc/pgx/v5/pgxpool#ParseConfig

Could also help: https://github.com/uber-go/goleak

jofmi commented 11 months ago

I have added another improvement that closes all rows after a db query and added additional error logs.

lebe1 commented 11 months ago

@lebe1 i have tried reproducing again with your video, with no success. i also tried to create a new item like it was already available in your video, which also created no issues. Can you see if the error still appears on your end if you use Aarons new branch augustin-wien/augustina-frontend#92? I also upgraded pgx to the latest version to see if this can fix your issue. Best i can suggest otherwise is that I take a look at the issue on your laptop directly.

Just tested changing the Mainitem with the new frontend branch and I still ran into the same db-connection-loss error. I could also reproduce the same db-connection-loss error by uploading a .jpg file instead of a suggested .png file. Besides this uploading a new logo, which is a .png file works like a charme. Hopefully, this is just a local issue on my machine and changing the main item is really not a common use case but maybe we stick all our heads together this friday to decide how to go further with this?

jofmi commented 11 months ago

Still no errors in the logs with the latest version of this branch?

nanu-c commented 11 months ago

deleted

lebe1 commented 10 months ago

Still no errors in the logs with the latest version of this branch?

Sadly no errors given besides the info No file passed or file is invalidhttp: no such file if I update the settings without an image. Still, it needs very few steps for me to run into this issue. Hopefully the production server does not as well since both of your machines did not have the problem... Therefore, I would give this one an approval to merge and create an issue to not forget about it.