Fix #65: ignore errors in tarballs/zipfiles #68
Conversation
|
Code is fine, but I'm not a fan of (almost) silently ignoring errors. Is there no way of communicating issues at all? How does it communicate invalid content? |
|
How the current system is setup, there are no persistent errors. So when you validate, it checks everything again to see if it works out okay now. This is how invalid content etc is detected. Tarballs and zipfiles are different, in that they are extracted on upload. There isn't any method of communicating any error here to the end-user. Basically, this PR resolves the reporting to Sentry about mistakes, but doesn't fix anything towards the end-user. I am also not sure how that would work / look like. I tried communicating back over the tus-protocol that there was an error, but it seems the javascript implementation completely ignores this, and still marks the upload as OK. I can experiment some more with this. But I am also fine closing this PR, so we can use Sentry to monitor how often people upload shitty stuff :P |
TrueBrain
force-pushed
the
fix_read_error
branch
2 times, most recently
from
6568628
to
094aa77
Compare
This is a bit of a hack in comparison with the normal validation method, but these errors happen on upload, where the other errors happen on validation. Sadly, the tusd implementation is a bit wonky, and we cannot really communicate back with the client when we found such error during upload (as strictly seen the upload succeeded fine; the content is broken). So we cannot report the error on upload, and as such are forced to do it after upload.
TrueBrain
force-pushed
the
fix_read_error
branch
from
094aa77
to
81583d0
Compare
There currently isn't really a way to talk back to the client that
they uploaded an invalid tarball/zipfile. So far this only happened
when someone uploaded a zero-byte tarball, which is pretty obvious.
Hopefully that remains this way.
The behaviour is now: if you upload an invalid tarball/zipfile,
after pressing validate, the file is no longer in the filelist.
Hopefully that gives enough clues to the user what is going on.