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

All http-client HEAD requests are marked as errors #2089

Closed
bcomnes opened this issue May 27, 2022 · 6 comments · Fixed by #2132
Closed

All http-client HEAD requests are marked as errors #2089

bcomnes opened this issue May 27, 2022 · 6 comments · Fixed by #2132
Labels
bug Something isn't working

Comments

@bcomnes
Copy link

bcomnes commented May 27, 2022

Expected behaviour
When making a HEAD request with the node http client, the dd-trace should properly detect and classify HEAD requests according to their success or failure.

Actual behaviour

In dd-trace 2.4.0, no HEAD requests are detected and in versions 2.4.1-2.8.0 all HEAD requests are detected, but classified as errors.

Steps to reproduce

Install dd-trace onto a node app and make HEAD requests with the http client where requestOptions.method = 'HEAD'.

It seems related to these changes which fixed a similar issue for all other http methods. Perhaps since HEAD requests don't deal with a body, it was unaddressed?

Environment

  • Operation system:
  • Heroku-20 stack
  • Node.js version:
  • ^17 || ^18
  • Tracer version:
  • 2.4.1-2.8.0
  • Agent version:
  • 7.36.0-1
  • Relevant library versions:

Package.json:

{
  "name": "gumcast-api",
  "description": "A service for generating rss feeds from gumroad subscriptions",
  "version": "1.2.12",
  "author": "Bret Comnes <bcomnes@gmail.com> (https://bret.io/)",
  "bin": {
    "gumcast-api": "api/bin.js"
  },
  "bugs": {
    "url": "https://github.com/bcomnes/gumcast-api/issues"
  },
  "engines": {
    "node": "^18.0.0 || ^17.0.0",
    "npm": "^8.3.1"
  },
  "dependencies": {
    "bent": "^7.0.7",
    "body-parser": "^1.19.0",
    "clean-deep": "^3.3.0",
    "cliclopts": "^1.1.1",
    "cors": "^2.8.5",
    "dd-trace": "2.8.0",
    "finalhandler": "^1.1.2",
    "http-hash-router": "^2.0.1",
    "http-proxy": "^1.17.0",
    "jsonfeed-to-rss": "^3.0.3",
    "lodash.get": "^4.4.2",
    "lru-cache": "^7.3.1",
    "minimist": "^1.2.0",
    "nanoassert": "^2.0.0",
    "p-connect": "^2.0.0",
    "p-map": "^4.0.0",
    "parseurl": "^1.3.3",
    "pino-http": "^7.0.0",
    "pino-pretty": "^7.3.0",
    "qs": "^6.7.0",
    "redirect-chain": "^1.0.1",
    "striptags": "^3.1.1",
    "util.promisify": "^1.0.0"
  },
  "devDependencies": {
    "auto-changelog": "^2.2.0",
    "dependency-check": "^4.0.0",
    "gh-release": "^6.0.0",
    "nodemon": "^2.0.2",
    "npm-run-all": "^4.1.5",
    "standard": "^17.0.0",
    "tape": "^5.0.1"
  },
  "homepage": "https://github.com/bcomnes/gumcast-api#readme",
  "keywords": [
    "gumroad",
    "podcast"
  ],
  "license": "MIT",
  "main": "servr",
  "private": "true",
  "repository": {
    "type": "git",
    "url": "git+https://github.com/bcomnes/gumcast-api.git"
  },
  "scripts": {
    "dev": "nodemon ./api/bin.js -- --config ./config.json",
    "dev:api": "nodemon ./api/bin.js -- --config ./config.json",
    "dev:client": "budo client/index.js",
    "start": "./api/bin.js",
    "test": "run-s test:*",
    "test:api": "tape api/**/*.test.js",
    "test:deps": "dependency-check package.json api/bin --verbose --no-dev -i pino-pretty",
    "test:standard": "standard",
    "release": "git push --follow-tags && gh-release -y",
    "version": "auto-changelog -p --template keepachangelog auto-changelog --breaking-pattern 'BREAKING CHANGE:' && git add CHANGELOG.md"
  }
}

Screen Shot 2022-05-27 at 9 09 29 AM

Screen Shot 2022-05-27 at 9 09 56 AM

@rochdev
Copy link
Member

rochdev commented Jun 1, 2022

I just tried sending a HEAD request and it worked as expected with a 200 status code, the error flag set to 0 and no error tags. Can you provide a reproduction snippet that results in the error you are seeing?

@bcomnes
Copy link
Author

bcomnes commented Jun 1, 2022

The code that was triggering this was requests made by this library:

https://github.com/tractr/redirect-chain/blob/master/index.js

Perhaps something about how this code is handling the request is triggering an error in dd-trace?

@rochdev
Copy link
Member

rochdev commented Jun 1, 2022

Can you provide a reproduction snippet using that library that would demonstrate the error?

@bcomnes
Copy link
Author

bcomnes commented Jun 1, 2022

Sure

@bcomnes
Copy link
Author

bcomnes commented Jun 2, 2022

Sorry my wife just gave birth. I will follow up with a reproducible example as soon as I can.

@bcomnes
Copy link
Author

bcomnes commented Jun 11, 2022

Ok sorry for the delay @rochdev

I put together a small demo that demonstrates the error:

https://github.com/ballpit/dd-trace-test/blob/master/index.js

Its just a bare bones web server that has one route:

https://dd-trace-test.herokuapp.com/redirect-chain?get=https://t.co/Pq8kh5jZOY

It takes a get query param, then uses HEAD requests to find the final destination if the URL has redirects, using the request-chain library.

Even though the HEAD requests do not encounter errors, dd-trace marks them as errors with no other context as to why it thinks its an error:

Screen Shot 2022-06-11 at 11 45 57 AM

Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants