Skip to content
This repository has been archived by the owner on Jan 25, 2020. It is now read-only.

gzip not supported? #97

Open
undoZen opened this issue Feb 2, 2015 · 15 comments
Open

gzip not supported? #97

undoZen opened this issue Feb 2, 2015 · 15 comments

Comments

@undoZen
Copy link

undoZen commented Feb 2, 2015

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
Copy link
Contributor

totherik commented Feb 2, 2015

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
Copy link

The npm registry recently made the same change

npm/npm#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
Copy link
Contributor

jasisk commented Apr 5, 2016

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
Copy link

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

@jasisk
Copy link
Contributor

jasisk commented Apr 5, 2016

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
Copy link

Oh, cool. Thanks. Will try it out.

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

@jasisk
Copy link
Contributor

jasisk commented Apr 5, 2016

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.

alexindigo added a commit to alexindigo/docker-proxy-npm that referenced this issue Apr 5, 2016
@jasisk
Copy link
Contributor

jasisk commented Apr 5, 2016

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
Copy link

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
Copy link
Contributor

jasisk commented Apr 6, 2016

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
Copy link

Thanks a lot.

@alexindigo
Copy link

@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
Copy link

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
Copy link
Contributor

jasisk commented Apr 8, 2016

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! 😀

@jasisk
Copy link
Contributor

jasisk commented Apr 8, 2016

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.

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

No branches or pull requests

5 participants