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

602 stuck in sorry to bother you in kiosk mode #603

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danelec-hpm
Copy link

As pointed out in #602 it would be really nice when running in kiosk mode, that the error message automatically try to reload the page. I set it to 10 seconds to not overload the server if several build monitor views are running at the same time.

@basil basil requested a review from jan-molak May 3, 2022 15:21
@basil
Copy link
Member

basil commented May 3, 2022

@dcendents Any concerns with this fix?

@jan-molak
Copy link
Member

jan-molak commented May 3, 2022

@dcendents / @danelec-hpm - it's a valid use case, but I'm not sure about the proposed solution.

I have a couple of suggestions:

  • I don't think this logic should be mixed with displaying the alert box; A better place to handle this would be in the jenkins:unknown-communication-error handler
  • When using timeouts, let's use AngularJS $timeout instead of setTimeout
  • In general, I'm not sure if 10-second timeout is sufficient; connection errors might happen when someone's restarting Jenkins, for example, and that could take longer than 10s; in which case, the user would end up with a blank screen
  • I'm not sure if auto-reload is the desired behaviour in all cases;

So let's take a step back and go to the original requirement from #602:

When updating Jenkins or on reboot (cookies timed out) the page will show the Sorry to bother you message, and be stuck at that message

@danelec-hpm - have a look at the handleLostConnection function; it already has a concept of an error counter.

However, when you're updating Jenkins or on reboot, it looks like it's the "unknown error" path that gets triggered.

I'd suggest investigating what error code is returned by Jenkins in your case - maybe we could add it explicitly to the lost connection handler?

If not, then we could change the handleUnknown(error) handler to increase the counter instead of bailing out straight away, and only when the number of errors exceeds some value (5? 10?) make it broadcast the failure.

@dcendents
Copy link
Contributor

yes I understand the need, we have the same problem here. When we reboot jenkins (install new plugins) it invalidates the crumbs and breaks the kiosk mode display.

Our current solution is a script in tampermonkey that reloads the tab every hour (and I try to update plugins at the end of the day or very early).

If we brainstorm a bit, I would think the best approach would be to keep trying to access jenkins in the background and also renew the crumb if necessary (there must be an API for that), then simply remove the alert box when everything is back to normal, without having to reload the page.

@jan-molak
Copy link
Member

If we brainstorm a bit, I would think the best approach would be to keep trying to access jenkins in the background and also renew the crumb if necessary (there must be an API for that), then simply remove the alert box when everything is back to normal, without having to reload the page.

Yup, I agree. Ideally, we shouldn't be bothering the user unless we really can't recover, but I wouldn't like to end up on a blank / 404 page if we reload before Jenkins starts up.

@basil
Copy link
Member

basil commented May 3, 2022

there must be an API for that

Yes, you are referring to the crumbIssuer API. There are examples of how to use it here:

https://support.cloudbees.com/hc/en-us/articles/219257077-CSRF-Protection-Explained

@danelec-hpm
Copy link
Author

danelec-hpm commented May 4, 2022

Only problem with retrying the connection in the background, is if the cookies or access token are invalid, only a reload will trigger new cookies to be generated. We see this issue if we shut down the kiosk PC over night and start again in the morning. The build monitor page then starts with this error, even though it's a fresh open of the page. Clicking the reload solves it.

@dcendents
Copy link
Contributor

I played with this a bit and managed to get a new CSRF crumb using javascript, however after a restart the URLs are not valid anymore.

e.g. the guid part is not valid anymore: /$stapler/bound/0c0c0284-3d2b-4621-9c5e-62248f43753a/fetchJobViews

I don't know if there's also an easy way to get a new valid URL, but if it is not possible at least we can use the crumbIssuer to test that jenkins has restarted.

On my (very slow) test server, with nginx reverse proxy and jenkins under the context /jenkins/, when I restart the server the following happens:

  1. fetchJobViews returns with 500 or 502 status code (also got a 404 once)
  2. fetchJobViews returns with 403 status code
    • if we reload the page here, I get the "Jenkins is restarting" page and will eventually open the main jenkins page (not the build monitor view)
  3. fetchJobViews still return 403 status code while crumIssuer returns 503 status code
  4. eventually I get a new crumb
  5. if I reload the page at this point I correctly land on the build monitor view

Here my quick and dirty code (Note the hard-coded /jenkins/ context in the URL)
(I'm not always using angular services as I was changing the javascript directly in chrome's developer console so could not change the angular config to inject more services)

                return function (error) {
                    switch (error.status) {
                        case 0:
                        case -1:
                        case 404:
                        case 500:
                        case 502:
                        case 503:
                            return handleLostConnection(error);

                        case 403:
                            // try to get a new crumb to test jenkins is up an running
                            // hard-coded /jenkins/ context
                            $http({
                                url: "/jenkins/crumbIssuer/api/json",
                                contentType: "application/json",
                                headers: {
                                    "User-Agent": "build-monitor"
                                }
                            }).then(function successCallback(response) {
                                console.log(response.data.crumb);
                                // althought we have a new crumb, $stapler URLs might have changed
                                // force a reload
                                window.location.reload();
                              });
                            return handleLostConnection(error);

                        case 504:
                            return handleMisconfiguredProxy(error);

                        default:
                            return handleUnknown(error);
                    }
                }

@dcendents
Copy link
Contributor

@danelec-hpm technically we don't need a cookie, we need a valid crumb header to fetch the json builds metadata. As long as the user session is still valid at the container level (tomcat) after a reboot, we can get a new crumb and could potentially resume from there. This might imply having the remember me checkbox ticked when logging in.

But as I said just before the URLs change after a restart, I'm really not familiar with $stapler to understand how it works.

But your change to force a reload after 10 seconds wouldn't work for me on my test server, I need more than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants