search

mailpile / Mailpile

A free & open modern, fast email client with user-friendly encryption and privacy features
https://mailpile.is
Other
8.81k stars 1.02k forks source link

Improve HTTP Error code messages in UI #2220

Closed Pectojin closed 5 years ago

Pectojin commented 5 years ago

When attempting to uploade images that are too large the backend correctly returns a 413 Entity Too Large error, but the frontend displays a very generic error message.

Returning the HTTP message would be pretty helpful :)

Screenshot 2019-06-04 at 19 00 25 Screenshot 2019-06-04 at 18 59 46
h3artbl33d commented 5 years ago

This is a good idea. But, I guess the 413 error came from your reverse nginx proxy?

Pectojin commented 5 years ago

Oh, I hadn't thought of that.

It may be the cause of the actual issue. I just compressed the images instead of investigating, though.

This issue is definitely only about the feature suggestion to improve visibility in the UI :)

h3artbl33d commented 5 years ago

I agree with that one. I am not a member of the Mailpile project, though I am trying to help out other users with issues and might contribute toward further development in the future.

The reason for spotting that this is most likely a nginx issue is rather simple: I ran a reverse proxy before and experienced the same issue. It also can occur if you try to upload an attachment that is too big.

I used this server block for putting nginx in front of Mailpile: mailpile.conf. It uses TLS and some securityheaders - on the local network.

It might be somewhat... paranoia. That is the way I roll.

BjarniRunar commented 5 years ago

Thank you for reporting this, but if I understand correctly, the issue is that nginx is generating an error condition that Mailpile does not expect, and so the UI is unhelpful?

I don't think it's reasonable to expect Mailpile to gracefully handle and interpret and explain all the errors that can be injected by intermediate proxies; the place to fix this is in the nginx config. So I'm going to close this. Please feel welcome to disagree and try to convince me otherwise if you feel I'm wrong about this. :smiley_cat:

Pectojin commented 5 years ago

I don't think it matters why the errors happens. We're seeing a notification say there is an error but refusing to say what the error is.

It doesn't have to do anything but pass the error up, such as changing from err.message to err.message err.code here: https://github.com/mailpile/Mailpile/blob/babc3e5c3e7dfa3326998d1628ffad5b0bbd27f5/shared-data/default-theme/html/jsapi/compose/attachments.js#L203

BjarniRunar commented 5 years ago

I totally agree and have merged your PR. Thank you for taking the time to discuss and contribute!

Pectojin commented 5 years ago

Thanks for dealing with me ;)