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

Feature Request/Idea: catch-all error pages #8450

Closed
eunices opened this issue Feb 24, 2022 · 11 comments
Closed

Feature Request/Idea: catch-all error pages #8450

eunices opened this issue Feb 24, 2022 · 11 comments

Comments

@eunices
Copy link
Contributor

eunices commented Feb 24, 2022

Overview of the Feature Request
Create a catch-all error page

What kind of user is the feature intended for?
All

What inspired the request?
Error pages reveal Payara version. It doesn’t provide user with support email to ask for help.

The curl command has response 405 and reveals Payara version error pages:
curl -X PUT https://$SERVER_URL
image

The curl command has response 501 and reveals Payara version error pages:
curl -X UNLOCK https://$SERVER_URL
image

What existing behavior do you want changed?
None

Any brand new behavior do you want to add to Dataverse?

• Create a generic, catch-all custom error page, that is similar to 404.xhtml

“Oops, something went wrong!” place of “ Page Not Found - The page you are looking for was not found.” (red bounding box in screenshot below)
“If you believe this is an error, please contact {0} for assistance.” should remain.

image

Under web.xml add:

<error-page>
   <location>/error.xhtml</location>
</error-page>

See reference: https://stackoverflow.com/questions/2237434/multiple-error-code-configuration-web-xml

@pdurbin
Copy link
Member

pdurbin commented Mar 2, 2022

Overall, I like this idea. I just thought I'd note that if I try

curl -X FOOBAR http://localhost:8080 (my dev laptop), I do get the Payara version, as explained above.

However, if I try

curl -X FOOBAR https://demo.dataverse.org I get the Apache version, which seems less bad to me. (Apache is easily upgraded with a package manager, typically.)

@eunices
Copy link
Contributor Author

eunices commented Mar 3, 2022

Just a quick input.

The result of curl -X FOOBAR https://<server> depends on the config of each installation's Apache. It may be better to standardize at the application level.

@eunices eunices closed this as completed Mar 7, 2022
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Up Next 🛎 to Done 🚀 Mar 7, 2022
@eunices eunices reopened this Mar 7, 2022
@eunices
Copy link
Contributor Author

eunices commented Mar 7, 2022

@pdurbin I'm sorry - I accidentally closed the issue and it got pushed to "Done" via automation. Kindly change back to "Up Next" where it was.

@sekmiller sekmiller moved this from Up Next 🛎 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 1, 2022
@sekmiller sekmiller assigned sekmiller and unassigned sekmiller Apr 1, 2022
@sekmiller sekmiller moved this from IQSS Team - In Progress 💻 to Up Next 🛎 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 4, 2022
@landreev landreev self-assigned this Apr 5, 2022
@landreev landreev moved this from Next Sprint to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 5, 2022
@landreev
Copy link
Contributor

landreev commented Apr 7, 2022

I tried adding an error page without the code specified, as described above, and I'm seeing some strange behavior. It is working for the first example - sending PUT to the top page; which results in a 405 "PUT is not supported". However, this catch-all solution does not in fact appear to be catching ALL errors. The next example, an attempt to use an illegal method ("-X FOOBAR") results in empty output (i.e., no error page or output of any kind - neither the Payara, nor the custom page I put in place), and the code 400, instead of 501 that was observed previously. (??) There is nothing in the server log. I can't immediately think of an explanation for this.

@landreev
Copy link
Contributor

landreev commented Apr 7, 2022

I'm beginning to question whether this is necessary. Or at least whether we want this error page to be modeled after our 404.xhtml; i.e. does it need to be a rich HTML page? After all, neither of the examples above would ever be sent from a user's browser in real life. So it should probably be plain text or basic json for exotic HTTP codes like that (?).

@pdurbin
Copy link
Member

pdurbin commented Apr 7, 2022

My 2 cents... whatever's easiest to implement! 😄

@landreev landreev moved this from IQSS Team - In Progress 💻 to This Sprint 🏃🏃‍♀️ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 11, 2022
@landreev landreev removed their assignment Apr 11, 2022
@landreev
Copy link
Contributor

landreev commented Apr 13, 2022

@eunices Hi, do you have any thoughts about the behavior I saw when I tried adding this "universal error page" to web.xml, described above? - that somehow changes the HTTP code returned in response to "-X FOOBAR", from 501 to 400; and no longer produces ANY output:

% curl -X FOOBAR "http://localhost:8080" 
%
(empty response)

with headers:

% curl -i -X FOOBAR "http://localhost:8080"
HTTP/1.1 400 Bad Request
Server: Payara Server  5.2021.7 #badassfish
X-Powered-By: Servlet/4.0 JSP/2.3 (Payara Server  5.2021.7 #badassfish Java/AdoptOpenJDK/11)
Content-Type: application/xhtml+xml;charset=UTF-8
Connection: close
Transfer-Encoding: chunked
X-Frame-Options: SAMEORIGIN

%

Have you tried it yourself, is there a trick ... ? I'm out of ideas really and didn't immediately find an answer online. And the Payara error page seems like a better option than no page at all...
(creating dedicated entries specifically for 400 and 501, in addition to this "catch-all" page doesn't help either)

@landreev
Copy link
Contributor

@eunices What is the use case for having a "pretty" html page for these, more exotic HTTP error codes anyway? I'm not dismissing it outright, just trying to understand. We maintain the custom pages for the 403, 404 and 500 codes because web users actually get these errors when using the site. With the codes like the "501 - method not defined" in the example above - would a regular user ever get that error in a browser? (I may be missing something of course).
(A simple alternative approach may be to try and put together one generic error page, but then maintain separate <error-page> sections in web.xml for the codes that are likely to be produced by Dataverse; all referencing the same generic page - ?).

@eunices
Copy link
Contributor Author

eunices commented Apr 19, 2022

Hi @landreev apologies for the slow reply. I'm not too familiar with the Github notifications and missed this one.

@eunices Hi, do you have any thoughts about the behavior I saw when I tried adding this "universal error page" to web.xml, described above? - that somehow changes the HTTP code returned in response to "-X FOOBAR", from 501 to 400

I did not tried these scenarios, it's strange that with and without headers the responses are different with the universal error page. It could be a bug.

(A simple alternative approach may be to try and put together one generic error page, but then maintain separate <error-page> sections in web.xml for the codes that are likely to be produced by Dataverse; all referencing the same generic page - ?).

I see, that could be fine as well, redirecting to a generic page for 501 and and 405, instead of a catch-all which seems to be buggy.

@landreev landreev moved this from This Sprint 🏃‍♀️ to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Apr 25, 2022
@landreev landreev self-assigned this Apr 25, 2022
landreev added a commit that referenced this issue Apr 25, 2022
it doesn't appear to be working for some http codes (see the issue, #8450)
@landreev
Copy link
Contributor

I took another shot at this. I checked in an example of adding this "generic page", not as a catch-all, but specifically for the code 405 (one of the 2 initial examples). (see the tagged branch) The approach does not work for the code 501 (you just cannot define a custom error-page for 501 - I have no idea why). I have not tried to find out which other return codes it works or does not work for.
As I said earlier, I'm not sure this is actually worth the effort. Especially since in real life error codes other than 403/404/500 are rarely seen by human users, then maybe the simple pages Glassfish or Apache are showing for these codes are actually preferable - ?

Considering moving this into community-dev for somebody else to take a whack at it.

@eunices
Copy link
Contributor Author

eunices commented Apr 26, 2022

Thanks @landreev for the update.
Yes, if a catch-all error page is buggy then it might not be worth the effort. Thanks for following up and testing.

@eunices eunices closed this as completed Apr 26, 2022
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from IQSS Team - In Progress 💻 to Done 🚀 Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

4 participants