Unreachable code after infinte loop inside generic function "http#serve"

[MaxGraey]

Code in this line never run and listener never closing.

[kevinkassimo]

Currently there is indeed no way of closing the listener. It would be a good idea to revamp the HTTP module with more careful design (and possibly emulating part of implementation from Go) (a few contributors have expressed interest on this)

[sholladay]

Yep, seems like this isn't possible right now. I was looking for a way to implement a server.close() equivalent in pogo, to gracefully stop accepting requests and allow the process to exit with a status code of zero (success). This is needed for production deployments, where killing a process from the outside can cause all kinds of problems.

[kevinkassimo]

For people who want this feature immediately, it is possible to add server.close feature given current http.ts structure. Basically you want to add something so that you can call and stops

deno_std/http/http.ts

Lines 60 to 69 in bef7ba1

	const acceptRoutine = () => {
	const handleConn = (conn: Conn) => {
	serveConn(env, conn); // don't block
	scheduleAccept(); // schedule next accept
	};
	const scheduleAccept = () => {
	listener.accept().then(handleConn);
	};
	scheduleAccept();
	};

(this piece of code immediately tries to wait for a next accept when the previous one is handled)
and also breaks from

deno_std/http/http.ts

Lines 74 to 85 in bef7ba1

	while (true) {
	await env.serveDeferred.promise;
	env.serveDeferred = deferred(); // use a new deferred
	let queueToProcess = env.reqQueue;
	env.reqQueue = [];
	for (const result of queueToProcess) {
	yield result;
	// Continue read more from conn when user is done with the current req
	// Moving this here makes it easier to manage
	serveConn(env, result.conn, result.r);
	}
	}

(This is for sure not an elegant structure. I hope someone could take some time to improve this while still keeping the capability of doing async iterator for request handling.)

[sholladay]

Maybe I could do something along the lines of:

class App {
    async handleRequest(request) {
        // ... do stuff with request ...
    }
    close() {
        this._shouldClose = true;
    }
    async start() {
        const server = http.serve('localhost:1234');
        for await (const request of server) {
            await this.handleRequest(request);
            if (this._shouldClose) {
                break;
            }
        }
    }
}

But I haven't tested it. And I think that would wait for a request to come in before closing, which seems bad.

[JoakimCh]

I'm testing a hack to support closing the server, preferably something with very few and simple changes.

For example this is a way you could run the server and handle it closing:

const generatorStopper = {stop: null}
const requestGenerator = http.serve(this.address, generatorStopper)

for await (const req of requestGenerator) {
  if (req === false) {
    log('Server has closed')
    break
  }
  // else handle the request
}

Where this would be called to stop it:

generatorStopper.stop()

And how the serve generator function could implement it (tested and works). I'm no TypeScript programmer, so please excuse my dirty JavaScript... :P

export async function* serve(addr: string, generatorStopper) {
  const listener = listen("tcp", addr);
  const env: ServeEnv = {
    reqQueue: [], // in case multiple promises are ready
    serveDeferred: deferred()
  };

  // Routine that keeps calling accept
  let handleConn = (_conn: Conn) => {};
  let scheduleAccept = () => {};
  const acceptRoutine = () => {
    scheduleAccept = () => {
      listener.accept().then(handleConn).catch(()=>{/*ignore*/});
    };
    handleConn = (conn: Conn) => {
      serveConn(env, conn); // don't block
      scheduleAccept(); // schedule next accept
    };

    scheduleAccept();
  };

  acceptRoutine();
  
  if (typeof generatorStopper == 'object' && 'stop' in generatorStopper) {
    generatorStopper.stop = (() => {
      env.serveDeferred.resolve(false);
    });
  }
  
  // Loop hack to allow yield (yield won't work in callbacks)
  while (true) {
    if (await env.serveDeferred.promise === false) {
      break
    }
    env.serveDeferred = deferred(); // use a new deferred
    let queueToProcess = env.reqQueue;
    env.reqQueue = [];
    for (const result of queueToProcess) {
      yield result;
      // Continue read more from conn when user is done with the current req
      // Moving this here makes it easier to manage
      serveConn(env, result.conn, result.r);
    }
  }
  listener.close();
  yield false;
}

Of course I modified a few other places in the code to support the new "false" type. E.g:

interface Deferred {
  promise: Promise<{}>;
  resolve: (arg) => void;
  reject: () => void;
}

And the listenAndServe function needs to be modified also, or just commented away.

If doing something like this then we need a way to handle "Uncaught Other: Listener has been closed". I have not tried to solve everything yet... I've just spent a few minutes at this! And if we need to connect to our own server for the socket.accept() function to stop holding, then that's an easy hack to fix that...
EDIT: Fixed the uncaught exception.

All I'm saying is that there are solutions to implement a way to close the server and it is not hard to do. But since I'm no TypeScript programmer I'm not really going to contribute with any pull requests any time soon, I don't have the energy/health to get involved in this yet.

EDIT: Added this text:
If you want to try these changes out then just create a "httpServer.js" file in your project directory and copy the contents of "https://deno.land/x/http/server.ts" into it. With these changes at the top (so your local file can fully replace the one at the server):

import { BufReader, BufState, BufWriter } from "https://deno.land/x/io/bufio.ts";
import { TextProtoReader } from "https://deno.land/x/textproto/mod.ts";
import { STATUS_TEXT } from "https://deno.land/x/http/http_status.ts";
import { assert } from "https://deno.land/x/testing/asserts.ts";

Add rest of changes and then import that file instead in your project:

import * as http from "httpServer.ts"