WorldWideTelescope / wwt_kernel_data_relay

A Jupyter Server extension enabling kernels to publish data to predictable URLs
MIT License
1 stars 2 forks source link

500 errors in CosmicDS #4

Open pkgw opened 2 years ago

pkgw commented 2 years ago

Reported by @Carifio24 and @patudom.

[W 11:11:49.762 NotebookApp] wwt_kernel_data_relay | incomplete or missing response from kernel | key=pywwt_static0.15.2.final.0 entry=research/ kernel_id=9e00bd6d-6dce-451f-9dbc-2fe254f67dd2 msg_id=eaaeed9b-adc0cfd1d5c6791fb3b8dc0e_1227763_53
[E 11:11:49.762 NotebookApp] {
 "Host": "localhost:8890",
  [...]
 "Referer": "http://localhost:8890/notebooks/cosmicds/stories/hubbles_law/CosmicDS.ipynb",
 }

"primarily when running our app in a Jupyter notebook (as opposed to voila). [W]e get a 500 error, and the window doesn't pop up."

pkgw commented 2 years ago

I haven't reproduced the 500 error but I can cause something to go wrong:

  1. Create custom Python environment for testing.
  2. Check out cosmicds/cosmicds to e00de24cac83df6439d9f1c4c11425d1ff6d5ea2 (last pre-prekdr commit)
  3. Work through pip install -e . with various version tweaks to get everything working
  4. jupyter notebook . in cosmicds directory
  5. Open cosmicds/stories/hubbles_law/CosmicDS.ipynb
  6. Click through intro

I eventually get:

[IPKernelApp] ERROR | No such comm target registered: jupyter.widget.control
[IPKernelApp] WARNING | No such comm: 2297d3b5-1e2c-453b-a0ee-f96089dd9273
[I 12:33:55.570 NotebookApp] 302 GET /api/kernels/wwtkdr/pywwt_static0.15.2.final.0/research/?origin=http%3A%2F%2Flocalhost%3A8888 (127.0.0.1) 0.550000ms
[I 12:34:11.275 NotebookApp] 302 GET /api/kernels/wwtkdr/pywwt_static0.15.2.final.0/research/?origin=http%3A%2F%2Flocalhost%3A8888 (127.0.0.1) 0.380000ms
[W 12:34:11.283 NotebookApp] 404 GET /api/kernels/wwtkdr/pywwt_static0.15.2.final.0/research?origin=http%3A%2F%2Flocalhost%3A8888 (127.0.0.1) 4.840000ms referer=http://localhost:8888/notebooks/cosmicds/stories/hubbles_law/CosmicDS.ipynb

... which obviously isn't what's supposed to be happening. Will investigate.

pkgw commented 2 years ago

OK, this error is due to the cosmicds monkey-patch removed in https://github.com/cosmicds/cosmicds/commit/9545c1cf79a1dfd03175c84a5b000d47f4ed0a70 . The message of the commit that added it, https://github.com/cosmicds/cosmicds/commit/d826882f45049774e93465e8ee037ef9cc5e523e, mentions that it's for the Voila baseurl, so if the monkeypatch was active when the code was being run in a plain notebook, that would potentially cause problems. But it would cause what I saw before, not the 500 errors reported above.

@nmearl Would this monkeypatch still be necessary when running pywwt KDR in Voila? We should find a way to fix the pywwt implementation to work correctly in as many circumstances as possible.

@patudom @Carifio24 I'll play around some more but it would definitely be helpful to get a reproduction script.

Carifio24 commented 2 years ago

@pkgw So I can sometimes generate a 500 error as follows:

Occasionally the window doesn't appear, with the body of the iframe being just the text incomplete or missing response from kernel

In the terminal, I get:

[W 18:05:43.050 NotebookApp] wwt_kernel_data_relay | incomplete or missing response from kernel | key=pywwt_static0.15.2.final.0 entry=research/ kernel_id=47c13242-7183-40a5-a6c2-2a97c8d4c7e2 msg_id=5d18a799-4fff484f8de9f466fef2996e_13335_44
[E 18:05:43.051 NotebookApp] {
      "Host": "localhost:8888",
      "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9",
      "Referer": "http://localhost:8888/notebooks/cosmicds/stories/hubbles_law/CosmicDS.ipynb",
      "User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36"
    }
[E 18:05:43.051 NotebookApp] 500 GET /wwtkdr/pywwt_static0.15.2.final.0/research/?origin=http%3A%2F%2Flocalhost%3A8888 (127.0.0.1) 30142.010000ms referer=http://localhost:8888/notebooks/cosmicds/stories/hubbles_law/CosmicDS.ipynb

This only happens occasionally. I'll update if I can come up with a better way to reproduce this.

pkgw commented 2 years ago

Thanks, that's helpful. Due to the way the KDR architecture works, it could definitely make sense that a kernel restart needs to be involved to trigger the issue.

pkgw commented 2 years ago

OK, cool, I can reproduce. Do you also get log messages like:

[IPKernelApp] WARNING | Unknown message type: 'wwtkdr_resource_request'
[IPKernelApp] WARNING | No such comm: 017dc4c439ff41a3ad3fdd9edfe8c0ad

when the problem occurs?

Carifio24 commented 2 years ago

I do get that message, but at least for me, it pops up together with the 302 -> 404 error.

pkgw commented 2 years ago

For the 302/404 error: after checking out the pre-prekdr commit, you need to edit cosmicds/__init__.py to remove the monkeypatch.

pkgw commented 2 years ago

OK, I think I understand what's going on here.

Basically, in this setup, when the Voila app is present and the WWT app has been loaded at all, and you restart the kernel, the browser needs to re-fetch the WWT app, and the request goes in before the new kernel has been set up to handle KDR requests. I think that this is due to rules about reparenting iframes.

Hypotheses:

  1. If you clear the output ("Cell" menu) so that the Voila app goes away, then restart the kernel, things should work.
  2. If you open the Voila app, do not navigate to a WWT page, then "Restart and Clear Outputs", then rerun, things should work.
  3. If you do the previous item but do navigate to a WWT page and do not subsequently clear the output cell, there will probably be issues, because of the re-fetch triggered upon cleaning up the Voila app. Even if you then navigate away from a WWT page.

So I think that right now, if one makes sure to always clear the output(s) before restarting the kernel, things should work consistently.

But, this is all stemming from the fact that the Python kernel is trying to provide the research app through the KDR. If the pywwt server extension is installed, the research app can be made available directly, without requiring any use of the KDR. Then things should Just Work™ much more reliably. The problem right now is that pywwt-in-the-kernel doesn't (and can't) know whether pywwt-in-the-server is available ... but I think I can figure out a way to allow it to detect that. Or I can at least give you an API to manually activate that mode.

pkgw commented 2 years ago

OK, I think https://github.com/WorldWideTelescope/pywwt/pull/331 should give you a way to address this. Once it's released, if you create all of your WWTJupyterWidget instances with the argument app_url = "/wwtstatic/research/", the problems here should be avoided, I think.

pkgw commented 2 years ago

@Carifio24 @nmearl If one of you gets the chance to test this out with the branched version of pywwt, that would make me more confident that it's a sufficient solution. But based on my testing I think it's OK so I'm happy to go ahead and merge and release to make the new API easily accessible.

edit: Also it would be nice to devise and merge a fix to remove the requirement for the Voila monkeypatch before I release the next version.

nmearl commented 1 year ago

@pkgw Revisiting this issue on a fresh JupyterHub install reveals that, with using app_url="/wwtstatic/research", the embeded iframes in the JupyterWWTWidgets still do not show WWT. It seems that the iframe url is http://localhost:1200/user/admin/wwtstatic/research, but the true WWT url (verified by visiting it directly in a local tab and having it render successfully) is http://localhost:1200/user/admin/user/admin/wwtstatic/research.

Any insight into why that second user/admin is being introduced?

This is with using the latest master of PyWWT (aea3f41) and the latest release of Voila.

pkgw commented 1 year ago

Weird! But also not too surprising, since the handling of the baseurl seems like something that is always going to be very finicky. I'll look into it.

pkgw commented 1 year ago

Hmmm. Well, I don't think that pywwt is doing anything wrong here. The code to set up the URL handling is very straightforward and pretty much verbatim out of the Jupyter docs. In an environment like BinderHub we get correct behavior with regards to the base_url setting.

So I think there must be an issue in JupyterHub, or maybe somehow the interaction of JupyterHub and Voila, that leads to this problem? Maybe the way that the "hub" functionality works, the user's Jupyter Server should think that it has a base_url of /, but it's getting /user/:user instead? If the hub applied the /user/:user prefix at a different layer of the HTTP processing, I think that could cause what you're seeing.

Anyway, lacking a JupyterHub install (and not coming up with anything that looks relevant on Google), I can't make much progress on this. @Carifio24 are you able to run JupyterHub and potentially dig into this?

nmearl commented 1 year ago

@pkgw Interesting. Yeah, it seems like if I override L84 will route_pattern = url_path_join('/', "/wwtstatic/(.*)"), the widgets render properly. The handler path must be getting set additionally somewhere else along the way.

pkgw commented 1 year ago

Yeah, I'd recommend trying to research whether the base_url setting there should be just '/' in the JHub environment. If that's the case, then the next step is to track down why it isn't. If it should have the /user/... form, then I think the issue would have to lie somewhere in the way that the JHub server is routing/proxying requests to the per-user servers.

nmearl commented 1 year ago

@pkgw So using the latest Voila, the issue has changed. Currently, it's now impossible to use Voila with PyWWT in any context, and I'm quite stumped. As an MVP:

  1. Create a new environment with Voila (latest from pip; 0.3.6 at the time of writing) and the latest PyWWT from main.
  2. (optional) Downgrade ipywidgets to avoid latest breaking changes: pip install "ipywidgets<8"
  3. Create a new notebook with
    from pywwt.jupyter import WWTJupyterWidget
    wwt = WWTJupyterWidget(app_url="/wwtstatic/research/", hide_all_chrome=True)
    wwt
  4. Run with voila Untitled.ipynb --Voila.base_url=/ --Voila.server_url=/ --Voila.ip=0.0.0.0 --Voila.tornado_settings="{'allow_origin': '*' }" --VoilaConfiguration.file_whitelist='[".*"]' --VoilaConfiguration.enable_nbextensions=True
  5. Notice 404 GET /voila/files/wwtstatic/research/ (127.0.0.1) 0.38ms. This is due to Voila redirecting. Talking with Maarten, he suggested manually controlling the redirect...
  6. Edit the handler.py file to circumvent redirect: vi $(python -c "import voila; print(voila.__path__[0])")/handler.py

       async def get_generator(self, path=None):
           # if the handler got a notebook_path argument, always serve that
           notebook_path = self.notebook_path or path
    
           if path is not None and 'wwtstatic' in path:           # Add line
               self.redirect(url_path_join(self.base_url, path))  # Add line
               return                                             # Add line
  7. Notice 304 errors in the console network log.
  8. ???

Any ideas would be very helpful.

pkgw commented 1 year ago

@nmearl I should be able to look into this in the next few days. I'll let you know what I find.

pkgw commented 1 year ago

By "the latest voila", do you mean its Git main branch? The 0.4.0aX alpha releases on GitHub? Version 0.3.6 on PyPI?

What's the latest version that did work (at least better than the most recent issues)?

nmearl commented 1 year ago

By "latest", I mean 0.3.6 on pypi. The most recent working versions that we actually tested were forked versions of Voila 0.3.1.dev and PyWWT 0.12.0.

pkgw commented 1 year ago

OK, there is going to be some Stuff to work through here. For full WWT functionality in the Voila standalone app we'd need it to support Jupyter Server extensions, which it just doesn't right now. This is issue voila-dashboards/voila#955, with a precursor in issue voila-dashboards/voila#495 — it's been discussed for more than three years at this point.

As noted in the current Voila issue, there have been no less than four pull requests aiming to implement this, none yet merged. The most recent is voila-dashboards/voila#592, but the latest activity in any of these was more than a year ago.

Presuming that the situation regarding Jupyter Server extensions isn't going to magically change in the near future, it looks like we have to make it so that the WWT widget can work as a pure, old-school nbextension, without relying on any of the server extension functionality. This should be OK so long as any data to be rendered in WWT don't rely on the kernel data relay functionality, which basically means no loading FITS files from pywwt. For the CosmicDS use case I think that won't be a problem.

On the other hand, if we have the development bandwidth to contribute the "ExtensionApp" support to Voila, it seems like that would make a lot of people happy and fix a longstanding limitation.

pkgw commented 1 year ago

For reference, Voila isn't even supporting "classic" notebook server extensions, let alone modern Jupyter Server extensions. It currently only supports pure-static "frontend" nbextensions that provide some extra files to the server and have some JS invoked on page load. I think that pywwt can still work within those constraints, but this framework is waaay behind the way that the rest of the Jupyter ecosystem is doing things.