lukeed / polka

A micro web server so fast, it'll make you dance! :dancers:
MIT License
5.43k stars 174 forks source link

Weird issue with sapper #157

Closed etclub closed 3 years ago

etclub commented 3 years ago

I use polka in my sapper app. In server.js I have something like

polka() 
        .use(function(req, res, next){
            console.log(`~> Received ${req.method} on ${req.url}`);
            next();
        })
        ...
/////////////// NOTE this route outside of sapper
   .get('/test/:id', function(req, res){
      console.log(req.params)
      res.end('test')
    })
//////////////
    .use(
        compression({ threshold: 0 }),
        sirv('static', { dev }),
        sapper.middleware()
    )
    .listen(PORT, err => {
        if (err) console.log('error', err);
    });

Everything works well locally. I can visit all the routes (outside or inside of sapper) and get the expected results.

Now I npm run build and deploy it on my server, which use nginx as reverse proxy.

Weird thing happens. When I visit mydomain/test/12345, I get 404 Not found on the page, and ~> Received POST on /test/12345 on the console. And if I comment out the sapper stuff

            // .use(
        //     compression({ threshold: 0 }),
        //     sirv('static', { dev }),
        //     sapper.middleware()
        // )

then I can get test on my page and 12345 on the console.

From the log on console, my nginx proxy setting should be OK.

etclub commented 3 years ago

Is there anything magic during npm run build?

lukeed commented 3 years ago

You should either be using polka@next or defining options.ignore for the sapper middleware.

In Polka stable, the use() blocks run before the get(). This means that sapper sees the stuff that's "outside of sapper" too. (I have zero idea why the method is being changed to POST, but that's definitely not something within Polka.)

If you use polka@next, then the order you define your routes in is respected.

etclub commented 3 years ago

Yes, I am using polka@next. The method being changed POST is my mistake in minimizing the workable example (I tested another POST route).

But I still not clear how to solve the problem. In fact I had moved the route after sapper, but it did not help.

lukeed commented 3 years ago

Sapper is greedy and wants to hold onto everything. Try options.ignore -- https://github.com/sveltejs/sapper/pull/326

Beyond that I would need a minimal reproduction.

etclub commented 3 years ago

Thank you for your quick help. But how to set options.ignore if my route is something like /:name/test/:id?

BTW, my route outside of sapper (say /test/:id) is unique, there Is no such route in sapper. And I still wonder why it works locally as expected by npm run dev.

lukeed commented 3 years ago

It doesn't matter if your route is inside / known / defined by sapper. The way sapper is designed, it tries to touch/terminate everything.

As mentioned in the PR, you can use a functional ignore and test to see that the current route is not something sapper should know about.

lukeed commented 3 years ago

Cannot help further without reproduction. Even then, this is not a Polka issue.

etclub commented 3 years ago

Thank you. Here is the MWE. I just added a route in server.js based on sapper-template.

And I found it works if run it locally, no matter by npm run dev or node __sapper__/build. But if I deploy the build to my server, I get 404 when visiting the route.

mwe.zip

etclub commented 3 years ago

BTW, this is the main part of my nginx setting

location / {
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header Host  $http_host;
        proxy_set_header X-Nginx-Proxy true;
        proxy_pass http://localhost:3000/;
        #proxy_redirect off;
    }
lukeed commented 3 years ago

I think you have another nginx conf file that's interacting with your server. This also explains why the app runs fine locally.

On a fresh box, I unpacked your mwe and got it running after a production build. Then I installed nginx and had this as my only configuration:

server {
    listen 8081;
    server_name localhost;

    location / {
        # root html;
        # index index.html index.htm;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header Host  $http_host;
        proxy_set_header X-Nginx-Proxy true;
        proxy_pass http://localhost:3000/;
    }
}

Then visiting http://localhost:8081/foo/test/bar was identical to http://localhost:3000/foo/test/bar (note the port change). Both responded with {"name":"foo","id":"bar"} – and the underlying Sapper app was working too.

Closing as it can't be reproduced – and I'm fairly confident it is an inherited and/or misconfigured nginx setting.

etclub commented 3 years ago

At first, I thought it may be nginx setting issue. But I could see on the console ~> Received GET on /foo/test/bar. So I ruled it out.

Now I tried such ngnix.conf (almost the default one)

user  nginx;
worker_processes  1;

error_log  /var/log/nginx/error.log warn;
pid        /var/run/nginx.pid;

events {
    worker_connections  1024;
}

http {
    include       /etc/nginx/mime.types;
    default_type  application/octet-stream;

    log_format  main  '$remote_addr - $remote_user [$time_local] "$request" '
                      '$status $body_bytes_sent "$http_referer" '
                      '"$http_user_agent" "$http_x_forwarded_for"';

    access_log  /var/log/nginx/access.log  main;

    sendfile        on;
    #tcp_nopush     on;

    keepalive_timeout  65;

    #gzip  on;

server {
        listen 80;
        server_name my.domain.com;

        location / {
                # root html;
                # index index.html index.htm;
                proxy_set_header X-Real-IP $remote_addr;
                proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
                proxy_set_header Host  $http_host;
                proxy_set_header X-Nginx-Proxy true;
                proxy_pass http://localhost:3000/;
        }
}
#    include /etc/nginx/conf.d/*.conf;
}

But it still does not work. I'm at my wit's end. So please help me check what is wrong with this conf. Thank you very much. I am using nginx/1.18.0 on linux.

lukeed commented 3 years ago

I'm not a nginx expert and the issue is most definitely not Polka (or even Sapper) related. Sorry.

That said, remember that nginx inherits configurations from multiple locations. I suggest a fresh start on a fresh VM, like I did. Good luck!

etclub commented 3 years ago

Thank you very much.

But I still doubt it is nginx conf issue. I can visit all the other routes and get the expected result. This means the nginx conf works well. Even when visiting /foo/test/bar, it shows on the console by the sapper app ~> Received GET on /foo/test/bar. This means the request is passed to sapper.

lukeed commented 3 years ago

What is the response? You're only describing the console.

etclub commented 3 years ago

No error, related message is ~> Received GET on /foo/test/bar. No {"name":"foo","id":"bar"}. But on the web page, I see 404.

Just now I also tried out

  1. comment out the route '/:name/test/:id' in server.js
  2. add [name]/test/[id].js

Now it works with my original nginx.conf. I can see on the console:

~> Received GET on /foo/test/bar
{ name: 'foo', id: 'bar' }

and on the page:

{"name":"foo","id":"bar"}
lukeed commented 3 years ago

I'm so lost. Your explanations jump around & consistently reference logs/output that wasn't presented in the mwe above

This is not a Polka or a Sapper issue. If it were, it would be constant & not tied to or dependent on OS, nginx configs, etc

I tried, but this is not a nginx support room. If you can put together a minimal reproduction that actually reproduces something, I'd be happy to look into it again & see what I can do.

etclub commented 3 years ago

Yes, I thought it would be consistent, you would reproduce and get the issue with the mwe. Now I don't know how to make such MWE.

As you see, all the logs/output in the mwe is console.log(~> Received ${req.method} on ${req.url}); and res.end(JSON.stringify(req.params)). That is all I can provide.