krakenjs / kappa

A hierarchical npm-registry proxy
Other
557 stars 66 forks source link

gzip not supported? #97

Open undoZen opened 9 years ago

undoZen commented 9 years ago

recently the chinese npm mirror https://registry.npm.taobao.org enabled gzip, then our kappa server got 500 when proxy to this site.

here's our kappa.json:

{
    "servers": [
        {
            "host": "127.0.0.1",
            "port": "7000"
        }
    ],
    "plugins": {
        "kappa": {
            "vhost": "npmcc.creditcloud.com",
            "rewriteTarballs": true,
            "paths": [
                "http://127.0.0.1:7001/",
                "https://registry.npm.taobao.org/",
                "https://registry.npmjs.org/"
            ]
        }
    }
}

do request with Accept-Encoding: gzip

node@NODE:~$ curl http://127.0.0.1:7000/watchify -H 'Host: npmcc.creditcloud.com' --compressed
{"statusCode":500,"error":"Internal Server Error","message":"An internal server error occurred"}node@NODE:~$
totherik commented 9 years ago

This codebase does not explicitly handle gzip, and it doesn't immediately appear to be supported in hapi's replay.proxy. We'll investigate.

sarsenault commented 8 years ago

The npm registry recently made the same change

https://github.com/npm/npm/issues/12196

This caused our Kappa registry to start throwing errors in Wreck (SyntaxError: Unexpected token) due to the way it handles content-encoding: gzip. They seem keen to make the switch permanent eventually...

jasisk commented 8 years ago

We were bitten by this yesterday, too. Published a new build at around 3am EST in the always-attempt-gzip branch. Published as 1.0.0-rc.14 (also in the rc dist-tag, installable as npm install kappa@rc).

Not yet merged into latest and master because I'll need to backfill some tests but I deployed it internally last night and alls well.

alexindigo commented 8 years ago

@jasisk Looks like npm is not aware of your published version: a year ago

jasisk commented 8 years ago

Looks like npm is not aware of your published version

That's just the latest dist-tag.

$ npm info kappa dist-tags
{ latest: '1.0.0-rc.11', rc: '1.0.0-rc.14' }
alexindigo commented 8 years ago

Oh, cool. Thanks. Will try it out.

I don't see that errors by the way, but just sudden slowness of all requests.

jasisk commented 8 years ago

I don't see that errors by the way, but just sudden slowness of all requests.

slowness by way of client? That could be the client retrying on error (the npm client retries requests a couple of times before it gives up). Your kappa logs will probably show a 500 or two each time it seems slow.

jasisk commented 8 years ago

Looking at the linked npm thread, it looks like they are now correctly parsing the accepts-encoding header. This was not the case when I hurriedly made the change last night (it was returning gzipped payloads regardless, depending on which edge machine you hit).

As a result, I may back out the change and implement it correctly. I'll follow up with some of the npm folks and see what they think.

alexindigo commented 8 years ago

Yeah, about that :)

this log level:

  "plugins":
  {
    "kappa":
    {
      "paths":
      [
        "REGISTRY/_design/app/_rewrite/",
        "http://registry.npmjs.org/"
      ],
      "logLevel": "debug"
    }
  }

seems to have no effect.

jasisk commented 8 years ago

We log out using the standard hapi logging events, consumable by good. The versions we use are way out of date but they still work.

Here's an example of how to configure for logging (you'll need to install good@^3 and good-console@^2 as mentioned in this example's README).

alexindigo commented 8 years ago

Thanks a lot.

alexindigo commented 8 years ago

@jasisk This is what I see in the logs:

kappa-0 (out): 160406/010855.682, ops, memory: 34Mb, uptime (seconds): 300, load: 0.53662109375,0.8486328125,0.9462890625
kappa-0 (out): 160406/010848.856, request, http://8bbd70ca5673:80: get /newrelic {} 304 (8456ms) 
kappa-0 (out): 160406/010848.807, request, http://8bbd70ca5673:80: get /configly {} 304 (8519ms) 
kappa-0 (out): 160406/010849.149, request, http://8bbd70ca5673:80: get /envar {} 304 (8195ms) 
kappa-0 (out): 160406/010848.814, request, http://8bbd70ca5673:80: get /bole {} 304 (8535ms) 
kappa-0 (out): 160406/010848.844, request, http://8bbd70ca5673:80: get /vary {} 304 (9469ms) 
kappa-0 (out): 160406/010858.488, request, http://8bbd70ca5673:80: get /minimist {} 304 (153ms) 
kappa-0 (out): 160406/010858.550, request, http://8bbd70ca5673:80: get /json-stringify-safe {} 304 (131ms) 
kappa-0 (out): 160406/010858.547, request, http://8bbd70ca5673:80: get /http-proxy {} 200 (265ms) 
kappa-0 (out): 160406/010858.660, request, http://8bbd70ca5673:80: get /individual {} 304 (213ms) 
sarsenault commented 8 years ago

Even though npm said they fixed all the changes, the older version of Kappa still doesn't work. The new version works flawlessly. The old version still fails, and I think it is due to how Kappa passes along the client npm version in the requests. It sounds like the npm team decided to simply revert to the old strategy by checking for older client versions, but if your local npm client is a recent enough version it will cause the server to assume it can serve gzipped content, even though the proxy itself can't handle it. I assume this means those using older Kappa installs will have it permanently broken (when using recent npm clients), but that is just a guess. Time to upgrade!

jasisk commented 8 years ago

Even though npm said they fixed all the changes, the older version of Kappa still doesn't work.

Yup. It's due to the fact that in the version of the npmjs.com registry prior to the one deployed the other night, gzip wasn't supported, so we didn't support it either. Additionally, we try to do as little munging of the original request as possible so we previously shuttled along the request headers basically as is (minus a few things like authorization headers, etc).

Interestingly, the npm client has supported gzip since 2014-03-18 with v1.4.5 (dep bump commit, npm-registry-client commit) but it was never enabled on registry.npmjs.com until the other day.

So the problem was really two fold: on the one hand, npm initially turned on gzip without checking the accept-encoding header (since the client has supported it in every version released in the last two years). This caused gzipped payloads suddenly, which is why kappa started 500ing. That's the first problem which, as you've pointed out, npm has fixed.

The second problem is in kappa itself. Because we aimed to not mutate the request much, we blindly passed along the accept-encoding header. However, having never had to worry about gzip payloads in the past, we didn't previously support parsing them.

So the immediate, quick fix was to strip out the accept-encoding header from the proxied request. However, having done that, we realized npm wasn't listening to it anyway. The second part was supporting gzip payloads in the response regardless of what we asked for in the request. So that's what we did.

At this point, with npm changing their implementation to consider accept-encoding, doing one or the other of the above is sufficient. I'll be stripping out the header munging shortly (if you and npm both support gzip appropriately, we may as well use it!).

So that's why, despite rolling back the change, old versions of kappa won't work. Hope that helps explain it! :grinning:

jasisk commented 8 years ago

Also, I'll be leaving this issue open until I strip out the header munging. We technically support gzip now but, with npm now respecting the accept-encoding header, we're not actually leveraging it.