Implement HTTP endpoint factory #1369

hacksparrow · 2018-05-28T20:20:33Z

Implements #1331.

Main changes:

Added a new package @loopback/http-server
Made changes to @loopback/rest to use @loopback/http-server.

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

hacksparrow · 2018-05-29T10:50:02Z

packages/http-server/src/http-server.ts

+      try {
+        await pEvent(this.httpServer, 'listening');
+        const address = this.httpServer.address() as AddressInfo;
+        this.restServer.bind('rest.port').to(address.port);


@bajtos I wanted to use RestBindings.PORT instead of rest.port, but RestBindings itself is undefined when {RestBindings} is imported into http-sever.ts - http-sever.ts is executed before the RestBindings object is created.

If there is a solution, do let me know.

IMO, HttpServer should not deal with dependency injection at all. Instead, it should provide properties like port, hostname and perhaps url that can be used by consumers to update any bindings as necessary.

class HttpServer { get port(): number { return this.address().port; } address: AddressInfo { return this.address() as AddressInfo; } }

Later in RestServer:

async start() { // create http server and start it, then: this.bind(RestBindings.PORT).to(server.port); }

bajtos · 2018-05-29T11:47:11Z

packages/http-server/package.json

+    "build:current": "lb-tsc",
+    "build:dist8": "lb-tsc es2017",
+    "build:dist10": "lb-tsc es2018",
+    "clean": "lb-clean loopback-core*.tgz dist* package api-docs",


Replace loopback-core*.tgz with loopback-http-server*.tgz. Same in verify script below.

bajtos · 2018-05-29T11:48:08Z

packages/http-server/package.json

+    "README.md",
+    "index.js",
+    "index.d.ts",
+    "dist/src",


I believe we need to include all dist* folders. Here is the canonical form we are using in other packages:

"files": [ "README.md", "index.js", "index.d.ts", "dist*/src", "dist*/index*", "src" ],

bajtos · 2018-05-29T11:58:04Z

packages/http-server/src/http-server.ts

+/**
+ * Object for specifyig the HTTP / HTTPS server options
+ */
+export type HttpOptions = {


HTTP can refer to both client and server. I am proposing HttpServerOptions for more clarity.

bajtos · 2018-05-29T11:59:58Z

packages/http-server/src/http-server.ts

+ * @class HttpServer
+ */
+export class HttpServer {
+  private restServer: RestServer;


My understanding is that HttpServer should be a low-level building block used by higher-level classes like RestServer, maybe GrpcServer, etc. In which case HttpServer must not depend on any specific high-level server, such dependency goes in a reverse direction inside the dependency graph.

bajtos · 2018-05-29T12:00:35Z

packages/http-server/src/http-server.ts

+  private restServer: RestServer;
+  private httpPort: number;
+  private httpHost: string | undefined;
+  private httpServer: Server;


Is it necessary to prefix all variables with http, considering that they belong to a class that already has http in the name?

bajtos · 2018-05-29T12:13:44Z

packages/http-server/test/integration/http-server.integration.ts

+  it('stops server', async () => {
+    const server = await givenAServer();
+    await server.start();
+    await server.stop();


Similarly here - I think you need to verify that the server is no longer listening on the given port, e.g. a request fails.

bajtos · 2018-05-29T12:15:32Z

packages/http-server/test/unit/http-server.unit.ts

+import {Application} from '@loopback/core';
+import * as assert from 'assert';
+
+describe('HttpServer', () => {


AFAICT, these tests are just duplicating integration tests - what's the point?

IMO, unit tests for an http server class makes little sense. You would have to mock core HTTP module, I think the complexity of that is bigger than the benefits of testing in isolation.

I am proposing to remove unit tests completely and move any relevant bits to integration tests.

I am proposing to remove unit tests completely and move any relevant bits to integration tests.

Yes, totally agree. I didn't feel OK as I was writing those tests.

bajtos · 2018-05-29T12:18:01Z

packages/rest/src/rest.server.ts

@@ -167,6 +166,15 @@ export class RestServer extends Context implements Server, HttpServerLike {
    this._setupRequestHandler(options);

    this.bind(RestBindings.HANDLER).toDynamicValue(() => this.httpHandler);
+
+    this._httpServer = new HttpServer(


We need to defer creation of HttpServer instance until start is called, so that we can pick any changes in HOST/PORT bindings.

An outline of a test case to write:

const server = new RestServer({port: 80}); server.bind(RestBindings.PORT).to(0); await server.start(); // get the actual port number and verify it's not 80

bajtos · 2018-05-29T12:19:11Z

packages/rest/src/rest.server.ts

-
-    const httpPort = await this.get<number>(RestBindings.PORT);
-    const httpHost = await this.get<string | undefined>(RestBindings.HOST);
-    this._httpServer = createServer(this.requestHandler);


Continuing my previous comment - this is the place where to instantiate HttpServer as a replacement for http.createServer. We need to preserve the logic obtaining the latest values for port & host.

Cross-posting #1369 (comment)

An outline of a test case to write:

const server = new RestServer({port: 80}); server.bind(RestBindings.PORT).to(0); await server.start(); // get the actual port number and verify it's not 80

bajtos · 2018-05-29T12:21:48Z

packages/http-server/README.md

+
+const app = new Application();
+const restServer = new RestServer(app);
+const httpServer = new HttpServer(restServer, {port: 3000, host: ''}, (req, res) => {});


This example will need to be updated once we remove the dependency from HttpServer to RestServer, see my comments below.

I think it also makes more sense to reorder constructor arguments to accept request handler as the first arg and options as the second.

I would like something along the following lines:

import {HttpServer} from '@loopback/http-server'; const httpServer = new HttpServer( (req, res) => res.end('Hello world'), {port: 3000} );

bajtos · 2018-05-29T12:23:26Z

packages/http-server/package.json

+  "license": "MIT",
+  "dependencies": {
+    "@loopback/dist-util": "^0.3.1",
+    "@loopback/rest": "^0.10.4",


Please remove this (circular) dependency.

bajtos · 2018-05-29T12:23:42Z

packages/http-server/package.json

+    "@loopback/build": "^0.6.5",
+    "@loopback/core": "^0.8.4",
+    "@loopback/testlab": "^0.10.4",
+    "@types/debug": "^0.0.30",


This looks like an unused dependency to me, please remove.

jannyHou · 2018-05-29T21:06:06Z

packages/http-server/package.json

@@ -0,0 +1,50 @@
+{
+  "name": "@loopback/http-server",
+  "version": "0.0.0",


I think the first version would be "0.0.1"?

raymondfeng · 2018-05-29T21:53:33Z

I don't know why we completely threw away the abstraction at https://github.com/strongloop/loopback-next/blob/2bddc47ccda56a50c52a8a92facd87e3a3f4944d/packages/http-server/src/common.ts. IMO, the PoC only requires some clean up to align with latest code in master.

I agree with @bajtos's comments that http-server should be only responsible for creating/starting/stopping http endpoints and exposing server address. No dependency on core/rest modules should exist.

hacksparrow · 2018-05-30T19:28:37Z

@raymondfeng, @bajtos suggested to keep this PR as simple as possible and build from there.

raymondfeng · 2018-05-30T19:54:33Z

@raymondfeng, @bajtos suggested to keep this PR as simple as possible and build from there.

I thought the idea was to decompose my original PR into a few steps and refine each of them as we go. If we can combine http-server and http-server-express from the PoC, realign with master, trim/improve the code and test coverage, it should have been more effective. The key is to provide a good divide of responsibility so that other modules don't have to deal with lower-level/common http processing, which includes:

Create http/https/... endpoints based on the config
Manage the lifecycle of http endpoints (start/stop)
Expose bound addresses
Marshaling/unmarshaling
Middleware

bajtos · 2018-05-31T07:39:11Z

@raymondfeng

@raymondfeng, @bajtos suggested to keep this PR as simple as possible and build from there.

I thought the idea was to decompose my original PR into a few steps and refine each of them as we go.

Yes, that's my idea too!

My general opinion is that we should treat spikes/PoCs as throw-away code and build the production implementation from scratch using test-driven/test-first development. Otherwise it's too easy to overlook that some parts of spike code that leaked into production are not in production quality and/or don't have enough test coverage.

In this particular case, the PoC was created with the assumption that we are going to support multiple server frameworks (Koa, Express), which introduced additional complexity to the API. For example, consider HttpFactory<REQ, RES, APP> - do we still need REQ and RES generic/template parameters?

So far, we were following the plan outlined in #1082 (comment):

A pair of (req, res) arguments was changed to a single httpContext object.

Rework of REST internals to support pluggable HTTP transports (Express, Koa, etc.)

Introduction of Endpoint classes that encapsulate creation of HTTP/HTTPs servers, etc.

Since there was no follow-up comment from you, we assumed you are ok with my proposal 🤷‍♂️

The key is to provide a good divide of responsibility so that other modules don't have to deal with lower-level/common http processing, which includes:

Create http/https/... endpoints based on the config
-Manage the lifecycle of http endpoints (start/stop)

Expose bound addresses

Marshaling/unmarshaling

Middleware

This is a good list that allows me to better understand your vision. I don't remember seeing such list before, it would be great to have it at a prominent place in your PoC PR or some other place where its easy to find. Having said that, the fault may be at my side too, these changes are in progress for more than a month.

Back to the topic: The first three items make perfect sense and I think that's exactly the scope of this pull request. I have concerns about putting marshaling/unmarshaling and middleware capabilities into our shared low-level HTTP stack. I'd like us to work on these two changes in a different pull request(s), so that the initial set of features can be landed faster, and then we can keep discussions about the remaining two items focused and in a single place.

Thoughts?

bajtos

You are making good progress 👍

bajtos · 2018-05-31T07:47:05Z

packages/http-server/src/http-server.ts

+        this._host = '127.0.0.1';
+      }
+    }
+    this._url = `${this._protocol}://${this.host}:${this.port}`;


I believe using private properties with public getters is sort of an anti-pattern in JavaScript/TypeScript.

I am proposing to either move the logic building this._url to url getter function, or remove url getter function and rename private _url to public url.

bajtos · 2018-05-31T07:48:24Z

packages/http-server/src/http-server.ts

+    }
+    if (process.env.TRAVIS) {
+      // Travis CI seems to have trouble connecting to '[::]' or '[::1]'
+      // Set host to `127.0.0.1`


-1

We cannot ship workarounds for Travis CI in code that will be used by LB apps in production!

If IPv6 localhost addresses are causing problems in Travis, then let's fix our tests to explicitly listen on 127.0.0.1 instead.

bajtos · 2018-05-31T07:51:45Z

packages/http-server/src/http-server.ts

+    this._port = this._address.port;
+    this._protocol = this._protocol || 'http';
+    if (this._address.family === 'IPv6') {
+      this._host = `[${this._host}]`;


In a host name in the form [${IPv6 addr}] a valid IPv6 address? I mean what if somebody calls tcp.connect(httpServer.port, httpServer.host) - shouldn't host remain the same value as provided in the config?

In general, I find it is suspicious that changes in the format needed for URL are stored in server properties that have more wider usage than just URL.

Ok, let's address the format when and where encountered. Will remove the formatting code.

bajtos · 2018-05-31T07:53:31Z

packages/http-server/test/integration/http-server.integration.ts

+        res.end();
+      },
+      {
+        port: 9000,


Using hard-coded port numbers in tests is a bad practice, because the selected port can be already used on that machine, for example by another run of the same test suite. It also makes it difficult to run tests in parallel.

Please always use port 0 and let the operating system pick an available port for you.

bajtos · 2018-05-31T07:58:35Z

packages/http-server/test/integration/http-server.integration.ts

+    await server.stop();
+  });
+
+  // @bajtos how do we test this?


For example, you can verify that request fails with an error having code set to string ECONNREFUSED.

expect(() => request(server.url)) .to.be.rejectedWith({code: 'ECONNREFUSED'});

bajtos · 2018-05-31T07:59:11Z

packages/http-server/test/integration/http-server.integration.ts

+    const server = await app.getServer(RestServer);
+    server.bind(RestBindings.PORT).to(0);
+    await server.start();
+    // @bajtos `_httpServer` of `server` is protected, how do we check the port and other details here?


Please see the earlier comments about the fact that HttpServer should not be updating any bindings.

@bajtos that's a RestServer instance. HttpServer does not do any sort of binding now, it just starts a server and exposes properties about that server.

I see. Since HttpServer does not deal with any bindings, this test does not belong to packages/rest. packages/rest is the place where to test this functionality - I would be surprised if there wasn't any existing test for that.

bajtos · 2018-05-31T08:24:24Z

packages/rest/src/rest.server.ts

-        }
-      });
-    });
+    return this._httpServer.stop();


Now that you are directly returning the promise provided by stop, this function no longer needs to be async.

- async stop() { + stop() { // Kill the server instance.

Will await this._httpServer.stop(), instead.

bajtos · 2018-05-31T08:25:36Z

packages/http-server/src/http-server.ts

+  /**
+   * Protocol of the HTTP / HTTPS server
+   */
+  public get protocol(): 'http' | 'https' {


Please define a shared type for 'http' | 'https' and use it all places where you are duplicating this union now.

E.g.

export type HttpProtocol = 'http' | 'https';

bajtos · 2018-05-31T08:26:25Z

packages/http-server/src/http-server.ts

+    this._address = this.httpServer.address() as AddressInfo;
+    this._host = this._host || this._address.address;
+    this._port = this._address.port;
+    this._protocol = this._protocol || 'http';


Please move this initialization to the constructor.

bajtos · 2018-06-04T12:07:51Z

packages/http-server/src/http-server.ts

+ */
+export type HttpServerOptions = {
+  port: number;
+  host?: string | undefined;


host? already allows undefined, no need to repeat that again in the type definition.

host?: string;

bajtos · 2018-06-04T12:10:05Z

packages/rest/src/rest.server.ts

+      port: options.port,
+      host: options.host,
+      protocol: options.protocol || 'http',
+    });


Please move the initialization of _httpServer to _setupRequestHandler method below.

bajtos · 2018-06-04T12:11:59Z

packages/http-server/test/integration/http-server.integration.ts

+    const server = new HttpServer(
+      (req, res) => {
+        res.end();
+      },


Please extract this request handler that's repeated in all tests into a shared helper function.

const server = new HttpServer( dummyRequestHandler, // ... );

bajtos · 2018-06-04T12:13:04Z

packages/http-server/test/integration/http-server.integration.ts

+      (req, res) => {
+        res.end();
+      },
+      {port: 0},


Would it make sense to make the port setting optional and defaulting to 0?

Intended usage:

const server = new HttpServer(requestHandler);

It would also allow us to simplify RestServerConfig - instead of copying all interface members, we could leverage inheritance.

interface RestServerConfig extends HttpServerConfig { // ... }

Thoughts?

raymondfeng · 2018-06-04T15:49:31Z

In this particular case, the PoC was created with the assumption that we are going to support multiple server frameworks (Koa, Express), which introduced additional complexity to the API. For example, consider HttpFactory<REQ, RES, APP> - do we still need REQ and RES generic/template parameters?

Since we have settled on Express, the generic parameters are not needed any more. Such type of cleanup is what I had in mind for this task.

Responsibility of the http module

Create http/https/... endpoints based on the config (implemented by my PR, https://github.com/strongloop/loopback-next/blob/2bddc47ccda56a50c52a8a92facd87e3a3f4944d/packages/http-server-express/src/http-server-express.ts#L61)
Manage the lifecycle of http endpoints (start/stop) (implemented by my PR, https://github.com/strongloop/loopback-next/blob/2bddc47ccda56a50c52a8a92facd87e3a3f4944d/packages/http-server/src/common.ts#L176)
Expose bound addresses (implemented by my PR, )
Marshaling/unmarshaling (TBD)
Middleware (TBD)

bajtos · 2018-06-05T11:49:29Z

packages/http-server/test/integration/http-server.integration.ts

+// License text available at https://opensource.org/licenses/MIT
+import {HttpServer} from '../../';
+import {Application, ApplicationConfig} from '@loopback/core';
+import {RestServer, RestBindings, RestComponent} from '@loopback/rest';


Please remove any references to @loopback/rest to avoid a cyclic dependency.

bajtos · 2018-06-05T11:50:45Z

packages/http-server/test/integration/http-server.integration.ts

+    const server = await givenARestServer({rest: {port: 80}});
+    await server.bind(RestBindings.PORT).to(0);
+    await server.start();
+    expect(server.getSync(RestBindings.PORT)).to.not.equal(80);


Please move this test to packages/rest.

Indeed, it belongs to packages/rest.

bajtos

Two more comments, the rest of the patch LGTM.

Please wait for @raymondfeng's approval before landing.

bajtos · 2018-06-05T15:20:10Z

packages/http-server/src/http-server.ts

+    this._port = (httpServerOptions && httpServerOptions.port) || 0;
+    this._host = (httpServerOptions && httpServerOptions.host) || undefined;
+    this._protocol =
+      (httpServerOptions && httpServerOptions.protocol) || 'http';


DRY.

if (!httpServerOptions) httpServerOptions = {}; this._port = httpServerOptions.port || 0; // ...

I think we can also shorten the argument name to simply options.

bajtos · 2018-06-05T15:21:05Z

packages/http-server/src/http-server.ts

+  /**
+   * Starts the HTTP / HTTPS server
+   */
+  public async start(port?: number, host?: string) {


Let's keep start parameterless please - both host and port should be configured at the time when the server is created. Unless there is a good justification why we need these two parameters?

hacksparrow · 2018-06-06T15:58:02Z

@raymondfeng All checks passed. Any comments?

raymondfeng · 2018-06-06T17:10:00Z

please investigate https://travis-ci.org/strongloop/loopback-next/jobs/388859072

raymondfeng · 2018-06-06T17:12:22Z

packages/http-server/src/http-server.ts

+   * Stops the HTTP / HTTPS server
+   */
+  public async stop() {
+    this.httpServer.close();


We should check if this.httpServer is set.

raymondfeng · 2018-06-06T17:13:35Z

packages/http-server/src/http-server.ts

+  /**
+   * Protocol of the HTTP / HTTPS server
+   */
+  public get protocol(): HttpProtocol {


Why bother to have getter for these properties? Do you want to enforce readonly?

Yes, readonly. What do you suggest?

Have you tried readonly keyword from TypeScript?

Thanks, will use that.

I think we can use readonly modifier for all properties excluding port, because port changes as a result of start() call. OTOH, maybe we should keep a readonly _port property to hold the requested port (e.g. 0) and then write a port getter that will either return the actual port reported by this._httpServer.address().port or fall back to this._port.

This has an important benefit that multiple start/stop cycles of a server configured to listen on port 0 will always pick a new port. At the moment, I believe subsequent start() calls will pick the temporary port number assigned by the operating system for the first start() call.

@hacksparrow Please add a unit-test to verify the behavior of our server in this edge case.

@raymondfeng even an undefined host will behave like port set to 0. If the host value is not set during instantiation, the HttpServer instance will store it as undefined, however, the underlying http/https will generate a host value for the server - something like ::.

Since url is dependent on host and port, the only property that can be implemented as a readonly would be protocol.

@bajtos I will add tests for both host and port.

raymondfeng · 2018-06-06T17:18:51Z

Please also fix https://travis-ci.org/strongloop/loopback-next/jobs/388859076

raymondfeng · 2018-06-06T17:44:21Z

packages/http-server/src/http-server.ts

+   */
+  public async stop() {
+    if (this.httpServer) {
+      this.httpServer.close();


Should we expose the state of the HttpServer, such as started: boolean?

Good suggestion, will add. Initially I had listening, but removed it.

raymondfeng · 2018-06-06T17:50:40Z

packages/http-server/src/http-server.ts

+  public async start() {
+    this.httpServer = createServer(this.httpRequestListener);
+    this.httpServer.listen(this._port, this._host);
+    await pEvent(this.httpServer, 'listening');


I wonder why we don't use a callback for httpServer.listen() and close(). For example:

import {promisify} from 'util'; public async start() { this.httpServer = createServer(this.httpRequestListener); await promisify(this.httpServer.listen)(this._port, this._host); this._address = this.httpServer.address() as AddressInfo; this._host = this._host || this._address.address; this._port = this._address.port; }

Good idea, will add.

Please note that pEvent will listen for error events too. As a result, server.start() returns a rejected promise if the server cannot be started.

The callback for httpServer.listen is not error-first, it's just a regular event callback for listening event. See https://nodejs.org/dist/latest-v10.x/docs/api/net.html#net_server_listen:

When the server starts listening, the 'listening' event will be emitted. The last parameter callback will be added as a listener for the 'listening' event.

@hacksparrow Please start by adding a test to verify that start() returns a rejected promise when the server cannot be started. Quoting from the same doc page:

One of the most common errors raised when listening is EADDRINUSE. This happens when another server is already listening on the requested port / path / handle.

In the test:

create and start a server listening on port 0 (let the OS pick an available port)

create another server, set the port number explicitly to the same value as reported by the server started in the previous step

verify that start() returns a rejected promise and the error has code set to EADDRINUSE - see http://shouldjs.github.io/#assertion-rejectedwith

Once we have that test in place, you can try to use the promisifed listen method and see how easy/difficult it is to correctly handle errors too (compared to what we have now).

@bajtos added the test.

@raymondfeng why have callbacks, when we can just await?

raymondfeng · 2018-06-06T18:06:38Z

packages/http-server/src/http-server.ts

+   * URL of the HTTP / HTTPS server
+   */
+  public get url(): string {
+    return `${this._protocol}://${this.host}:${this.port}`;


Please make it more robust per https://github.com/strongloop/loopback-next/blob/2bddc47ccda56a50c52a8a92facd87e3a3f4944d/packages/http-server/src/common.ts#L176-L201

+1 to handle IPv6 localhost URLs correctly, although I am fine with leaving that out of scope of this pull request. Isn't there any Node.js core API or perhaps an npm module that can solve this problem for us?

The built-in url API seems to work well

$ node > require('url').format({protocol: 'http', hostname: '::1', port: 80, pathname: '/foo'}) 'http://[::1]:80/foo'

The docs says this code is considered as legacy and should not be used in new code :(

Unfortunately the newer WhatWG API does not encode IPv6 local addressed as the old one, so I am not sure how much benefit we can get from using it.

See nodejs/help#1176 (comment)

const u = new (require('url').URL)('http://./'); u.protocol = 'https'; u.host = '[::1]'; u.port = 3000; u.pathname = '/foo'; u.href // returns // 'https://[::1]:3000/foo'

-1 for Travis-specific hacks, see #1369 (comment)

We cannot ship workarounds for Travis CI in code that will be used by LB apps in production!

If IPv6 localhost addresses are causing problems in Travis, then let's fix our tests to explicitly listen on 127.0.0.1 instead.

@raymondfeng can we do the "robustification" after landing the basic functionality?

If we force tests to only listen on 127.0.0.1, we then lose the coverage for host === undefined.

Makes sense. I am proposing to write two tests for host === undefined case:

One test to verify what is returned by host and url properties. I think we don't need to start the server for this and thus these tests can be at unit-test level.

One test to verify that server listening on all hosts accepts connections from 127.0.1. The trick is to build the request URL manually (http://127.0.0.1:${server.port}/) in the test, don't use server.url property.

bajtos

LGTM as the first increment. I see few more opportunities to clean up the code, but that can be easily done later.

hacksparrow · 2018-06-08T16:04:24Z

@slnode test please

hacksparrow · 2018-06-08T16:20:20Z

@slnode ok to test

* Decouple the creation of HTTP/HTTPS server from the `rest` package. * Create new package (`http-server`) to handle the creation of HTTP/HTTPS server.

hacksparrow requested review from bajtos and raymondfeng as code owners May 28, 2018 20:20

hacksparrow commented May 29, 2018

View reviewed changes

bajtos suggested changes May 29, 2018

View reviewed changes

bajtos reviewed May 29, 2018

View reviewed changes

jannyHou reviewed May 29, 2018

View reviewed changes

bajtos suggested changes May 31, 2018

View reviewed changes

hacksparrow force-pushed the package/http-server branch from f95ab29 to b455b33 Compare June 3, 2018 21:24

bajtos suggested changes Jun 4, 2018

View reviewed changes

hacksparrow force-pushed the package/http-server branch from b455b33 to e1a8f71 Compare June 5, 2018 10:47

bajtos suggested changes Jun 5, 2018

View reviewed changes

hacksparrow force-pushed the package/http-server branch 2 times, most recently from 13d4f8e to c24e20b Compare June 5, 2018 15:04

bajtos approved these changes Jun 5, 2018

View reviewed changes

hacksparrow force-pushed the package/http-server branch 4 times, most recently from 5d44eaa to 1890eb3 Compare June 6, 2018 15:27

hacksparrow force-pushed the package/http-server branch from 1890eb3 to 0c0f1fd Compare June 6, 2018 16:34

raymondfeng reviewed Jun 6, 2018

View reviewed changes

hacksparrow force-pushed the package/http-server branch from 0c0f1fd to 5cffb71 Compare June 6, 2018 17:23

hacksparrow force-pushed the package/http-server branch from 5cffb71 to c322380 Compare June 6, 2018 17:40

raymondfeng reviewed Jun 6, 2018

View reviewed changes

hacksparrow force-pushed the package/http-server branch from c322380 to 1f89ee5 Compare June 7, 2018 20:53

bajtos approved these changes Jun 8, 2018

View reviewed changes

feat: add http-server package

bac8d8c

* Decouple the creation of HTTP/HTTPS server from the `rest` package. * Create new package (`http-server`) to handle the creation of HTTP/HTTPS server.

hacksparrow force-pushed the package/http-server branch from 1f89ee5 to bac8d8c Compare June 8, 2018 16:52

hacksparrow merged commit bac8d8c into master Jun 8, 2018

bajtos deleted the package/http-server branch June 11, 2018 09:34

dhmlau mentioned this pull request Jun 11, 2018

HTTP endpoint factory #1331

Closed

hacksparrow self-assigned this Jun 18, 2018

hacksparrow mentioned this pull request Jun 21, 2018

Handle IPv6 localhost URLs in http-server package #1453

Closed

Implement HTTP endpoint factory #1369

Implement HTTP endpoint factory #1369

Conversation

hacksparrow commented May 28, 2018

Checklist

hacksparrow May 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng commented May 29, 2018

hacksparrow commented May 30, 2018

raymondfeng commented May 30, 2018

bajtos commented May 31, 2018

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacksparrow May 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

bajtos Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacksparrow commented Jun 6, 2018

raymondfeng commented Jun 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng commented Jun 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng Jun 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng Jun 7, 2018 • edited Loading

hacksparrow May 29, 2018 •

edited

Loading

hacksparrow May 31, 2018 •

edited

Loading

bajtos Jun 5, 2018 •

edited

Loading

raymondfeng Jun 6, 2018 •

edited

Loading

raymondfeng Jun 7, 2018 •

edited

Loading