[Response] No Response Body Captured

chetsangnil commented 8 years ago

In my super basic node application, a request was successfully executed. Texts were visible in the browser but Glimpse client did not show anything in response body tab. Here's my code for this section:

app.get('/chet', function (req, res) {
  res.send('Hello Chet!' + '<html><head>HUD Test</head><body>This is a HUD test</body></html>');
  console.log("NodeJs Works");
});

In the Debug tab, here's the message for web-response:

{
    "url": "http://localhost:3333/chet",
    "headers": {
        "x-glimpse-contextid": [
            "4a817b306fcc11e6ac0b41de37da9755"
        ],
        "x-powered-by": [
            "Express"
        ],
        "content-type": [
            "text/html; charset=utf-8"
        ],
        "content-length": [
            "772"
        ],
        "etag": [
            "W/\"304-qBlMEO+qGD9duB+6/uFbPQ\""
        ]
    },
    "statusCode": 200,
    "duration": 4.795834,
    "endTime": "2016-08-31T15:43:09.544-0700",
    "body": {
        "size": 0,
        "encoding": "utf8",
        "content": ""
    }
}

"content" doesn't seem to capture any value. Please take a look into this.

philliphoff commented 8 years ago

Looking at this a bit more closely--it's by design. Express defaults to using using ETag's, which allow the server to determine whether the browser already has (cached) the content about-to-be returned by the server. If so, then the server alters the response to a 304 and omits the body, and the browser will simply use the body from its cache.

In my testing, in all cases where Glimpse captures a request returning 200 it also correctly captures the response body. In all cases where Glimpse captures a request returning 304, the response body is empty (as expected).

mike-kaufman commented 8 years ago

For this, is there something in the UI that we can show that more clearly indicates this was a 304 hence no body? Chet's expectation here is not unreasonable.

mike-kaufman commented 8 years ago

/cc @avanderhoorn @nikmd23 - would be curious to know what your experience was with glimpse v1.

Feel free to remove the beta/early beta tags if we feel this is too big.

philliphoff commented 8 years ago

@chetsangnil had created #8 related to indicating when no value--in particular, no body--exists. I imagine that this could piggypack on that. E.g. a (!) icon next to a standard No body text that, when clicked, shows a text bubble that indicates that a 304 response isn't expected to have a body.

nikmd23 commented 8 years ago

I think there may be two issues here.

The first, as @chetsangnil articulated, I think should be solved by #8.

However, when I look at the message being generated, I'm surprised to see the status code is 200. In ETag situations, a 304 should be returned (which means "hey client, use the content you have in your cache, I'm not going to re-send it"). A 200 would indicate "hey client, what you have is stale, use this instead".

So it looks like we're showing a 200 when we should be showing a 304?

@chetsangnil, can you confirm by comparing what Glimpse is showing to what your browsers dev tools show? I can walk you through how to do that.

philliphoff commented 8 years ago

Totally my bad; must have been a long day. I kept reading @chetsangnil's response ETag (which happened to start with 304) as the status code (listed directly below it). Copying his handler into my own sample app, I too see 200 responses with no body content captured by Glimpse (despite Chrome DevTools saying/showing a body was delivered).

I don't see this in the sample Sticker app, however.

philliphoff commented 8 years ago

In HttpProxy.ts it seems like we're calling the HTTP server handling logic before attaching/patching the request/response events/functions, which may introduce a race condition that could miss response data events. This seems to work for the Sticker sample app, but not for this simpler app. Maybe our sample tends to have large enough bodies to win the race?

If I move the invocation of the callback to after the subscription/patching, then Glimpse captures the body as expected. It looks like that code has been unchanged since @nebrius added body capturing, so he may have the best idea as to the intent of that ordering.

nikmd23 commented 8 years ago

Ping @nebrius ;)

nebrius commented 8 years ago

thanks @nikmd23. I noticed that this repo doesn't drop notifications in slack, can you update that?

nebrius commented 8 years ago

Ok, looking at the code, I see what you're talking about. There's no specific reason it's ordered the way it is other than historical reasons, so it's fine to bump it to the bottom AFAIK. We should of course test for regressions.

nikmd23 commented 8 years ago

Sorry it took me two weeks @nebrius, but this repo is now configured in Slack

Glimpse / Home

[Response] No Response Body Captured #7