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

optional await on server.run() #199

Closed
2 tasks
crookse opened this issue Apr 22, 2020 · 1 comment · Fixed by #202
Closed
2 tasks

optional await on server.run() #199

crookse opened this issue Apr 22, 2020 · 1 comment · Fixed by #202
Assignees

Comments

@crookse
Copy link
Member

crookse commented Apr 22, 2020

Summary

What: Implement logic so that the following syntax works:

class HomeResource extends Drash.Http.Resource {
  static paths = ["/"];
  public GET() {
    this.response.body = "Hello World!";
    return this.response;
  }
}

const server = new Drash.Http.Server({
  response_output: "text/html",
  resources: [HomeResource]
});

await server.run({ // the await here should be optional
  hostname: "localhost",
  port: 1447
});

console.log("server running");

console.log(server);

server.close()

console.log("server stopped");

This requires run() and runTLS() to use an IIFE when awaiting for requests from the Deno server.

Benchmarks were initially run on the implementation below and there was a small performance gain. Around ~2k req/sec gain.

Why: Syntax allows for cleaner implementation in cases where the server needs to be awaited on before running any subsequent code. This also ensures the developer that the server is running before any subsequent code is processed.

Acceptance Criteria

  • Documentation showing optional await keyword on server.run() and server.runTLS()
  • Tests pass without being refactored

Example Pseudo Code (for implementation)

  /**
   * @description
   *     Run the Deno server at the hostname specified in the configs. This
   *     method takes each HTTP request and creates a new and more workable
   *     request object and passes it to
   *     `Drash.Http.Server.handleHttpRequest()`.
   *
   * @param HTTPOptions options
   *     The HTTPOptions interface from https://deno.land/std/http/server.ts.
   *
   * @return Promise<void>
   *     This method just listens for requests at the hostname you provide in the
   *     configs.
   */
  public async run(options: HTTPOptions): Promise<void> {
    if (!options.hostname) {
      options.hostname = this.hostname;
    }
    if (!options.port) {
      options.port = 1447;
    }
    this.hostname = options.hostname;
    this.deno_server = serve(options);
    (async () => {
      for await (const request of this.deno_server) {
        try {
          this.handleHttpRequest(request);
        } catch (error) {
          this.handleHttpRequestError(request, this.httpErrorResponse(500));
        }
      }
    })();
    return this.deno_server;
  }

  /**
   * @description
   *     Run the Deno server at the hostname specified in the configs as an
   *     HTTPS Server. This method takes each HTTP request and creates a new and
   *     more workable request object and passes it to
   *     `Drash.Http.Server.handleHttpRequest()`.
   *
   * @param HTTPSOptions options
   *     The HTTPSOptions interface from https://deno.land/std/http/server.ts.
   *
   * @return Promise<void>
   *     This method just listens for requests at the hostname you provide in
   *     the configs.
   */
  public async runTLS(options: HTTPSOptions): Promise<void> {
    if (!options.hostname) {
      options.hostname = this.hostname;
    }
    if (!options.port) {
      options.port = 1447;
    }
    this.hostname = options.hostname;
    this.deno_server = serveTLS(options);
    (async () => {
      for await (const request of this.deno_server) {
        try {
          this.handleHttpRequest(request);
        } catch (error) {
          this.handleHttpRequestError(request, this.httpErrorResponse(500));
        }
      }
    })();
    return this.deno_server;
  }
@Guergeiro
Copy link
Member

I agree with it. While in my testing environment, sometimes it happened that I curl(ed) faster than the server start.

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

Successfully merging a pull request may close this issue.

2 participants