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

uws examples are misleading #5

Open
bompus opened this issue Aug 8, 2017 · 3 comments
Open

uws examples are misleading #5

bompus opened this issue Aug 8, 2017 · 3 comments

Comments

@bompus
Copy link

bompus commented Aug 8, 2017

While using uws yields some good performance benefits, trying to actually use it results in some issues.

The first issue I came across, is a fairly important one. Trying to add response headers in both Express and Koa do nothing. All that comes back is the Content-length header, nothing else.

const http = require("uws").http;
// const http = require("http");

Koa:
ctx.set("Cache-Control", "no-cache");
ctx.body = "Hello World";

Express:
res.set("Cache-Control", "no-cache");
res.send("Hello World");

If you use the native http require, it works just fine. Because of this behavior, I don't think you can do a simple 1:1 comparison and have uws claim victory, when it's doing less work.

Perhaps I am doing something wrong, so please check for yourself and let me know if you see any way around this. I know that from digging around, the author of Total.js was going to try uws as well, but I believe realized it had this same issue ( https://blog.totaljs.com/blogs/news/20170530-new-total-js-framework-version-2-6/ - A support for another HTTP module )

@hbakhtiyor
Copy link
Owner

hbakhtiyor commented Aug 9, 2017

the uws module is not fully compatible with native http native, you'll be face some missing properties/methods/events.

you can open issue at the repo if not working the above examples.

about the above blog, yeah, no has the property yet, but you can use alternative approach,
e.g.

'use strict';

const express = require('express');
const http = require('uws').http;

const app = http.getExpressApp(express);

app.post('/', (req, res) => {
  const body = [];
  req.on('data', (chunk) => {
    body.push(Buffer.from(chunk));
  }).on('end', () => {
    res.end('You posted me this: ' + Buffer.concat(body).toString());
  });
});

http.createServer(app).listen(8000, () => {
  console.log('Example app listening on port 8000!');
});

@bompus
Copy link
Author

bompus commented Aug 9, 2017

Interesting. So we end up parsing our own post body, trying to figure out how to send response headers, etc. Yeah, I see how that would cause issues with some frameworks that are expecting http.

Hopefully uws moves along and shims everything up to match, then I'd be curious to see some real 1:1 testing. I'm sure it is still way faster, but not being able to simply drop it in as a replacement halts me from using it at this time.

Thanks for putting together the benchmarks. Didn't want the issue to seem like a bash, just caught my eye immediately when I started testing the idea out :)

@hbakhtiyor
Copy link
Owner

Anytime ;)

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

2 participants