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

web-server/servlet/web seems to inhibit stateless operation #126

Open
shriram opened this issue Jul 12, 2023 · 15 comments
Open

web-server/servlet/web seems to inhibit stateless operation #126

shriram opened this issue Jul 12, 2023 · 15 comments

Comments

@shriram
Copy link

shriram commented Jul 12, 2023

I had a rough time getting stateless servers to work. The culprit appears to be web-server/servlet/web. Include it and stateless does not work, exclude it and it works quite seamlessly on a non-trivial system. See

https://discord.com/channels/571040468092321801/1128483815787077692

for a small, illustrative example, reproduced below.

However, I don't see anything in the documentation for either stateless or web-server/servlet/web that indicate this. It should be documented in both places.

Sample program:

#lang web-server

(require web-server/servlet-env
     web-server/servlet/web)

(define (start req)
  (println "starting")
  (let ([r (send/suspend
        (lambda (k-url)
          (response/xexpr
           `(html
         (body
          (form ([action ,k-url])
            (input ([type "text"] [id "uid"] [name "uid"]))))))))])
    (println "there")
    (let ([uid (extract-binding/single 'uid (request-bindings r))])
      (if (member uid '("hello"))
      (response/xexpr `(html (body (p "yes"))))
      (response/xexpr `(html (body (p "no"))))))))

(serve/servlet start
               #:listen-ip #f
               #:stateless? #t)
@jeapostrophe
Copy link
Contributor

web-server/servlet/web is in the "Stateful Servlets" section of the docs and provides the stateful versions of functions like send/suspend. Documentation for those functions mention that the servlets can expire --- which stateless ones cannot --- see https://docs.racket-lang.org/web-server/servlet.html#%28def._%28%28lib._web-server%2Fservlet%2Fweb..rkt%29._send%2Fsuspend%29%29

In contrast, web-server/lang/web is in the "Stateless Servlets" section and provides the stateless version of the same interface. This module is provided directly by #lang web-server, so you don't normally need to require it, as the last paragraph of the first section of the docs says --- https://docs.racket-lang.org/web-server/stateless.html

@jbclements
Copy link
Contributor

I agree that the documentation is as you state. However, I also think that if we're okay with flummoxing Shriram and everyone less racket- and technically minded than him (including me, I think... I tried once to use stateless servets and failed), we're probably not providing something that's as useful as it could be.

I'm wondering how hard it would be to actually signal an error here. My ham-fisted idea is to simply set a global parameter that causes these primitives to signal an error. Or are there situations in which you'd want to use the stateful primitives in the stateless server?

@jeapostrophe
Copy link
Contributor

It is not an error. You may want to mix them when you need to capture a continuation in code that is not transformed by #lang web-server

@jbclements
Copy link
Contributor

Ah well, I tried.

@shriram
Copy link
Author

shriram commented Jul 13, 2023

@jeapostrophe you answered the second part of @jbclements's post and completely ignored the important part, which is the first part.

@jeapostrophe
Copy link
Contributor

I don't understand what a reply to the first part would look like. I read it as, "Yes, the documentation is accurate, but it is hard to use, huh?" I don't what problem John had using it, but I guess the solution can't be "add more writing", because I assume documentation in Racket is too subtle already. (You search for send/suspend and don't notice what module you clicked on and don't notice what section you're in.)

It would be possible to rename send/suspend in the stateless one to send/suspend/but/dont/save/anything/on/the/server, but the whole point of #lang web-server is to be as compatible as possible to the stateful world.

It would be possible to add a warning through some runtime stuff in web-server/servlet/web that prints a warning if required at the same time... but that's weird to my taste. We don't do that when you "accidentally" require racket/class1.5 and racket/class3.7 and racket/class0.2 in the same code.

So... what do you want? I feel like John is inviting me to think of ideas but I don't have any. I am cursed with the knowledge of how to actually use it.

@LiberalArtist
Copy link
Contributor

One concrete idea: I had almost suggested to @shriram create-none-manager, but I checked the docs and read, "If you do use a continuation capturing function, the continuation is simply not stored. If the URL is visited, the instance-expiration-handler is called with the request." I think it would be useful to provide a way to get an error when a stateful continuation is captured, as opposed to when you (attempt to) restore it. I think you could do that with the current manager interface, as long as continuation-store! is passed a real continuation? (the docs are not explicit) that could be used with continuation-marks and continuation-mark-set->context.

Some other thoughts:

  1. It would be good to understand why @shriram's original stateful continuations were expiring so quickly and provide (or update) guidance accordingly. In particular, @Bogdanp mentioned that

    In congame, we set the memory threshold quite high:

    https://github.com/MarcKaufmann/congame/blob/da9d3d3074668c23ca16947e37f94805ab890d1d/congame-web/components/app.rkt#L244

    Because we found that, even with a relatively small number of students using the server concurrently, continuations were expiring very quickly (but we make extremely heavy use of continuations in congame).

    Maybe the guidance turns out to be that the default (make-threshold-LRU-manager #f (* 128 1024 1024)) manager is only suitable for small examples. Maybe the guidance should be pushing people more strongly toward stateless servlets.
    Personally, I was spooked enough by the warnings about memory use with stateful servlets that I've only really used stateless ones, even in cases when it probably didn't matter. But I know (e.g. from @shriram and @jeapostrophe's papers) that people have used stateful servlets for real applications, and I would have expected the resource usage of native continuations to be improved with Racket CS.

  2. One particular point of confusion was that timeouts associated with "safety limits", e.g. #:response-send-timeout, don't affect continuation expiration. We should probably at least mention that in the safety limits docs. (Maybe other places, too?)

  3. Multiple people (e.g. @soegaard) mentioned difficulty figuring out how to serve the paths they wanted. It sounds like some of that confusion was about serve/servlet's options like #:servlet-path, #:servlet-regexp, #:server-root-path, #:extra-files-paths, #:servlets-root, and #:servlet-current-directory. Many of those aren't passed on to serve/launch/wait and dispatch/servlet (what I tend to use), and some of them seem to be related to legacy options like dynamically loading servlets. Maybe there are documentation improvements to be made? Maybe we should provide a new function like serve/servlet, but with more streamlined options for the kinds of configuration people actively want these days in new code?

  4. Very broadly, the web server has a long history, and sometimes the documentation for historical pieces is too prominent relative to the currently-recommended ways of doing things. For example, the sections on both Stateful Servlets and Stateless Servlets begin by saying that servlets should provide exports including interface-version and start, but this is not necessary if you are using serve/servlet or dispatch/servlet in the generally-recommended ways. I've thought about attempting an overhaul of the documentation, but so far time constraints have gotten in the way. Incremental improvements are definitely possible, though.

  5. In part this is a particular example of the above, but I think https://docs.racket-lang.org/web-server-internal/Troubleshooting_and_Tips.html could use some attention:

    • In addition to the key length issue @shriram reported in update key length #125, in the course of racket/racket-pkg-website@43d955f I found that openssl genrsa has been deprecated in favor of openssl genpkey, which seems to have an automatic default length and avoids the password-stripping step.
    • We probably should add an example of using SSL with serve/servlet instead of plt-web-server, since plt-web-server is no longer recommended.
    • Ideally, we might figure out a way to test this in CI, or at least do so as part of the release process.
    • A nice enhancement would be to add guidance (and maybe code) for using Let's Encrypt certificates. I've written some code to handle certificate updates, but it's not as robust as I'd like, and I think I'd approach it differently today. Personally, the last time I was organizing a big project, I used Let's Encrypt on the server and plain HTTP on localhost, controlled by configuration or command-line options.
    • I was reminded while writing this of an issue with the Apache recommendations I'll report separately.
    • It would be nice for some Nginx user to contribute similar recommendations.
    • Maybe we should say more about the choice between terminating SSL in Racket or at the proxy?

@LiberalArtist
Copy link
Contributor

the default (make-threshold-LRU-manager #f (* 128 1024 1024)) manager

(Well, that's the default for serve/servlet: the default for dispatch/servlet is (make-threshold-LRU-manager #f (* 1024 1024 64)).)

@shriram
Copy link
Author

shriram commented Jul 16, 2023

These are all good ideas @LiberalArtist. Practically, I think it would be nice to do at least these three things:

  1. The key issue is what @jeapostrophe points out: context.

    The docs are confusing in this respect in general: it's really easy to get lost in where you are in the docs. But often this matters only inasmuch as you have scroll up to find a relevant include.

    Here, we have the somewhat unusual problem in that there are multiple APIs for the same behavior. (As Jay says, renaming the operators is not what you want: it's really handy to be able to switch between stateful and stateless. For debugging, for instance, it's handy to know you can go back to stateful easily and confirm things are working, then just flip the switch.) that in human factors is called "mode confusion": you have multiple modes but they all look the same so you don't know which mode you're in.

    Perhaps, at least, the relevant (or all) functions can indicate which mode they are in?

    And in particular, perhaps web-server/servlet/web can explicitly say "don't use this in stateless"? I honestly don't know what it does: "web-server", "servlet", and "web" are all sufficiently generic/meaningless names that there isn't even a naming hint that it might be problematic. Likewise, perhaps the stateless docs can contain a warning to remove any libraries like this that are problematic?

  2. I do agree it would be super-nice to give simple, step-by-step instructions on how to use Let's Encrypt. There's no real reason to have self-signed certs (which are anyway treated a bit sus in some contexts) when we can just do it right.

  3. Paths. I really still don't understand them. I just used enough options until I got something working. I believe there isn't an example anywhere that helps you with a really basic sort of thing you'd want to do: put an image in your root directory and serve it up as a /logo.png. I think I spent at least an hour, and way more than that in perceived frustration, over that. Twice I gave up on having a logo at all until someone (I think @LiberalArtist) gave me one more suggestion.

@shriram
Copy link
Author

shriram commented Jul 16, 2023

Even better, perhaps libraries that are known to be problematic with stateful or stateless can explicitly check and error? Is that too hard?

@racket-discourse-github-bot

This issue has been mentioned on Racket Discourse. There might be relevant details there:

https://racket.discourse.group/t/do-not-understand-stateful-vs-stateless-servlets/2220/2

@shriram
Copy link
Author

shriram commented Aug 19, 2023

OMG this is amazing!!!

@mfelleisen
Copy link
Contributor

The conversation here and some references to Discourse suggest that this is a problem of the developer's "state of mind" (paraphrasing @shriram here, w/o saying that these people are in a "confused" mode of thought).

So how about using "teaching libraries" (the analogue of teaching languages)?

With prefix-in we could easily produce variants of these libraries (incl. the language libraries) that signal which function belongs to which package: sf: for "state-full" and app: for "applicative" or "stateless" (if you prefer to characterize objects through the absence of properties).

Since the docs are written in Scribble, we can abstract over the content and produce a "beginner" version where every function name is prefixed. There is no question which function belongs where.

Advanced developers -- with tons of paged-in experience -- can ignore the prefixed variants and simply use the plain ones. They can switch from one mode to another with the change of a couple of lines for debugging. They can mix and match. They can read the non-prefixed docs and, when their modes of thoughts become confused, they can go back to the prefixed docs. (Perhaps the two should be interlinked with margin notes.)

(In general, I think *SL could benefit from teaching levels for packages but in this context, prefixing is too hard.)

@shriram
Copy link
Author

shriram commented Aug 20, 2023

So now if you want to switch briefly between the two — eg, stateless is not working, so you want to confirm that at least in stateful it works okay so the error isn't with your application logic — you have to find and change every app: to sf:, and later, vice versa. If you forget to switch some, I wonder what would even happen under this proposal?

@mfelleisen
Copy link
Contributor

No, see variant -- assuming you're an experienced developer.

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

No branches or pull requests

6 participants