internetarchive / openlibrary

One webpage for every book ever published!
https://openlibrary.org
GNU Affero General Public License v3.0
5.13k stars 1.34k forks source link

Add debug button/link to error page (only for librarians) #8881

Open RayBB opened 7 months ago

RayBB commented 7 months ago

Describe the problem that you'd like solved

This was discussed on the community call. When some navigates to a page with an error they see something like this:

image

If you append ?debug=true to the url then you can see a stacktrace with more detailed information.

I propose that we add a button or link that only shows up for librarians that will take them directly to the debug page.

Proposal & Constraints

Though in many cases it could be as simple as appending the ?debug=true we want to make sure if there are other queryparams (such as during search) that we append appropriately with &debug=true.

This should be done in the python code of the error template.

Additional context

Related docs: https://github.com/internetarchive/openlibrary/wiki/Debugging-and-Performance-Profiling

The code for this template lives in the infogami repo here: https://github.com/internetarchive/infogami/blob/3a6508785487728911252a9df53bcc5f44a4cde1/infogami/utils/template.py#L173-L183

Stakeholders

QuantuM410 commented 7 months ago

Hi @RayBB ! I'd like to collaborate on this, as I've encountered the same error before and would be happy to contribute to its improvement.

RayBB commented 7 months ago

@QuantuM410 I've assigned you. Good luck!

QuantuM410 commented 6 months ago

Hey @RayBB! I tried looking up the error template's python code and am a little confused. Are we talking about the openlibrary/infogami/utils/template py file? And if that's the case how can I build it to check the changes after implementation . . Moreover I think a url link should be sufficient instead of a button :)

RayBB commented 6 months ago

@QuantuM410 thanks for looking into it. You are right that the template is located here.

👍 on the url instead of button.

I posted a question about this on Slack here to get input from folks who are more familiar with this part of the system.

Pasting my message here for ease of use:

Need some input on this issue about changing the error template to show a link to the debug page but only for librarians. Here are my questions: Is it possible to do this in infogami? I’m not sure if infogami has access to data about if the request comes from a librarian. It seems messy to have it there. Also not sure if our fork of infogami is used by anyone else? How would we feel about doing this client side with JS? if error page && is librarian then show the debug link Seems like the most simple approach and I think we are OK with requiring JS for librarian things like this? Alternatives? Or maybe it just isn’t wroth the effort to implement this?

RayBB commented 6 months ago

@QuantuM410 would you still like to work on this? If so, was the information posted in Slack enough? If not, let me know so someone else can work on it.

QuantuM410 commented 6 months ago

@RayBB I read the thread and was a bit confused, now instead of a button or a link do we have to directly redirect the librarians to the debug page if there is an error?

RayBB commented 6 months ago

@QuantuM410 we should go directly to that page for librarians. Do you think that's feasible based on the code you've looked at so far?

QuantuM410 commented 6 months ago

@QuantuM410 we should go directly to that page for librarians. Do you think that's feasible based on the code you've looked at so far?

Well, as @mekarpeles explained in the thread on slack. That seems like an appropriate approach. Furthermore, I think it wraps up the step for librarians to debug the error instead of just being able to see the error info since we directly redirect them to the debug page making it more feasible? https://github.com/internetarchive/openlibrary/blob/9a5bd10e3ff8ebbf9158e4d4ff0e2bcbf0b71cc9/openlibrary/plugins/openlibrary/code.py#L962-L981 @RayBB If I am not wrong, according to the discussion we plan to add another case here checking if the user is librarian and then raise web.debugerror()

RayBB commented 6 months ago

@QuantuM410 that sounds about right to me. Lets try to get a PR going to test it 👍

QuantuM410 commented 6 months ago

@RayBB I have made a PR. Can you please check if this is what we were going for?