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

Aborted requests show status code 200 in response event #3878

Closed
clcpolevaulter opened this issue Nov 15, 2018 · 4 comments
Closed

Aborted requests show status code 200 in response event #3878

clcpolevaulter opened this issue Nov 15, 2018 · 4 comments
Assignees
Milestone

Comments

@clcpolevaulter
Copy link

@clcpolevaulter clcpolevaulter commented Nov 15, 2018

Notice that the response event below has a statusCode of 200 even though the request was aborted by the client. When logging this event, the status code becomes misleading when troubleshooting. Is there a way to set the status code to 499 instead? This seems related to #3561.

{
  "event": "response",
  "timestamp": 1539281741125,
  "id": "1539281741125:localhost:70560:jn4wlokx:10000",
  "instance": "http://localhost:8083",
  "method": "get",
  "path": "/version",
  "query": {},
  "responseTime": 4022,
  "statusCode": 200,
  "pid": 70560,
  "httpVersion": "1.1",
  "source": {
    "remoteAddress": "127.0.0.1",
    "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36"
  },
  "route": "/version",
  "log": [
    {
      "request": "1539281741125:localhost:70560:jn4wlokx:10000",
      "timestamp": 1539281742112,
      "tags": [
        "request",
        "error",
        "abort"
      ],
      "channel": "internal"
    },
    {
      "request": "1539281741125:localhost:70560:jn4wlokx:10000",
      "timestamp": 1539281745146,
      "tags": [
        "handler",
        "error"
      ],
      "error": {
        "data": "adam",
        "isBoom": true,
        "isServer": true,
        "output": {
          "statusCode": 500,
          "payload": {
            "statusCode": 500,
            "error": "Internal Server Error",
            "message": "An internal server error occurred"
          },
          "headers": {}
        },
        "isDeveloperError": true
      },
      "channel": "internal"
    }
  ],
  "config": {}
}

Also, notice the actual request failed even though the status code is showing as 200. 😕

Route handler looks like:

{
    config: {
        auth: false,
        handler() {
            return new Promise(function (resolve, reject) {
                setTimeout(function () {
                    reject(new Error('adam'));
                }, 4000);
            });
        }
    },
    method: 'GET',
    path: '/version'
}

I cancel the request from the browser before it finishes to produce the error.

Context

  • node version: 8.11.3
  • hapi version: 17.5.4
  • hapi good version: 8.1.1
  • original issue: outmoded/discuss#729
@hueniverse hueniverse closed this in 2f64d40 Jan 6, 2019
@hueniverse hueniverse self-assigned this Jan 6, 2019
@hueniverse hueniverse added this to the 18.0.0 milestone Jan 6, 2019
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Jan 6, 2019

@kanongil @dominykas would appreciate your feedback on this. The braking change is that this will now log close/abort as errors by default and would require log configs to change to ignore the 499 status code if people want to simulate the old setup which reported these as 200 because the response was forced to null when not transmitted.

I think this is the right way to handle it, as close/abort events are errors that should be logged and monitored as indication that something bad is going on with your users if there are too many of these.

@kanongil
Copy link
Contributor

@kanongil kanongil commented Jan 7, 2019

I'm fine logging these as errors. Code 499 seems a bit strange, but I guess it is ok. Alternatively, a code 0 might work as well to indicate that nothing was sent.

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Jan 10, 2019

boom is not going to like code 0. I used 499 to match nginx which was a good suggestion. We can add a special boom error for this and set it to a special code (maybe non-numerical), but that seems a bit over the top. I am going to add a flag to the error to allow checking that instead of error code.

@lock
Copy link

@lock lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants