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.

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

Updating pre-existing file in public folder without caching fails and produces 412 #1189

Open
rosano opened this issue Jul 15, 2020 · 9 comments

Comments

@rosano
Copy link
Contributor

rosano commented Jul 15, 2020

I resolved this via storageClient.caching.enable('/public/MODULE_NAME/') and if I just write a second time it works, but I'm wondering why it is necessary to cache the potentially large public folder to write to it.

Maybe related to #887 or maybe I'm not handling conflict events properly like in #826

@raucao
Copy link
Member

raucao commented Jul 15, 2020

I'm wondering why it is necessary to cache the potentially large public folder to write to it

That's not the case per se (and definitely not intended). Sharesome uses zero caching and only the public folder for example. But it doesn't overwrite existing files. So it could very well be a bug with 412 handling in public directories.

If it works on the second try, then it sounds very much like there's a conflict on the first try. You would see the 412 status in the network tab of your browser console then.

@raucao
Copy link
Member

raucao commented Jul 15, 2020

@rosano Which caching strategy did the code use when you encountered this behavior?

@rosano
Copy link
Contributor Author

rosano commented Jul 15, 2020

@skddc I always use caching.enable which defaults to ALL, but this was not on the public folder.

If it works on the second try, then it sounds very much like there's a conflict on the first try. You would see the 412 status in the network tab of your browser console then.

Yes this happens every first try when caching is not enabled for the public folder. I guess for me it's strange to call it a 'conflict' because I always intend to write to it without caring what's there, and it seems like the 'most recent wins' doesn't apply in this case.

@raucao
Copy link
Member

raucao commented Jul 15, 2020

That would be a bug then. But it could also be a bug on the server side. Does it actually send an ETAG to check for conflicts in the PUT request headers?

Edit: It would be the If-Match header. If the server's version differs from what is sent, then it returns the 412.

@rosano
Copy link
Contributor Author

rosano commented Jul 15, 2020

Here are the headers I can see in Safari:

Screen Shot 2020-07-15 at 16-2

@galfert
Copy link
Member

galfert commented Jul 15, 2020

The If-None-Match: * header means "only create the file if it doesn't exist on the server yet". That's why the server is responding with a 412.

Looks like a bug to me. The library doesn't know of the already existing file and thinks it's creating a completely new one.

So when caching is disabled, I think it should not send that header. But that would put the responsibility of making sure to not overwrite any existing files unintentionally to the app, not the library.

@rosano
Copy link
Contributor Author

rosano commented Jul 15, 2020

But that would put the responsibility of making sure to not overwrite any existing files unintentionally to the app, not the library.

I wonder if it's merely an edge case to treat the public folder as "always writeable without concern about conflicts" or if it is worth making a flag for this somewhere in the library? If the app primarily writes to the private folder, then public is usually just a secondary copy that can be safely overwritten – from a UX perspective, I would imagine that the person using the app sees it that way as well.

@raucao
Copy link
Member

raucao commented Jul 15, 2020

I'm wondering if we need some kind of force-overwrite argument for the PUT functions, in order to ensure that implementors are aware of the situation.

@rosano
Copy link
Contributor Author

rosano commented Jul 15, 2020

I'm wondering if we need some kind of force-overwrite argument for the PUT functions, in order to ensure that implementors are aware of the situation.

Oh yes it would be even more clear if it happened on a per-call basis, not globally as I had imagined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants