fix: Ignore HTTP store client errors trying to get content length #2874
Conversation
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
changed the title
vrongmeal
changed the title
| "{err}\n\ | ||
| Note that globbing is not supported for HTTP.", |
There was a problem hiding this comment.
I would avoid newlines in errors.
There was a problem hiding this comment.
The error is usually very long. Moreover, we just want to append a "note" here for the user (easier to read).
There was a problem hiding this comment.
I mean, I think it's fine.
If it's very long it's going to wrap anyway and adding our own new line character isn't going to actually be a great place in all cases.
My concern is that it's going to end up in log messages with a new line that's going to be hard to read and maybe hard to parse.
I'd also change it to put the summary at the beginning: HTTP sources do not support globbing: {err}
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
|
A test fails with a less precise error, should we just let it be for now. There isn't a better work around to improve the error message... |
|
It looks like we're removing tests here - are there any tests we could add? |
The `HEAD` method might not be supported. Ignore client side errors and assume we have `0` content length. Fixes: #2828 Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
vrongmeal
force-pushed
the
vrongmeal/http-fix
branch
from
70fcffe to
82a198e
Compare
So, to make most of the things work, we had to do a bit of hacky stuff which messes up the error messages. So the tests removed are simply not checking some particular error messages (related to HTTP). We still have the same tests below checking another error message so our coverage is still the same. |
The
HEADmethod might not be supported. Ignore client side errors and assume we have0content length.Fixes: #2828