Skip to content

Conversation

trentm
Copy link
Member

@trentm trentm commented Jan 18, 2023

The 'http.request' instrumentation fix in #3090 uncovered a bug in our
tests for @hapi/hapi when with node v8 (a version before
http.request(url, opts, cb) was supported in node). This fixes that.

This also updates the .tav.yml section for '@hapi/hapi' to align with
logic in "_is_hapi_incompat.js" so the TAV tests don't waste time
installing a version of Hapi for which tests will be skipped anyway.

Refs: #3090


A TAV test failure example is here: https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-nodejs%2Fapm-agent-nodejs-mbp/detail/main/578/pipeline/2844
The test hanged:

.ci/scripts/test.sh -b "release" -t "@hapi/hapi" "8" 
...
[2023-01-18T07:05:32.394Z] node_tests_1  | -- required packages ["@hapi/[email protected]"]
[2023-01-18T07:05:32.394Z] node_tests_1  | -- installing ["@hapi/[email protected]"]
[2023-01-18T07:05:47.859Z] node_tests_1  | -- running test "node test/instrumentation/modules/hapi/basic.test.js" with @hapi/hapi
[2023-01-18T07:05:49.066Z] node_tests_1  | (node:136) UnhandledPromiseRejectionWarning: TypeError: "listener" argument must be a function
[2023-01-18T07:05:49.066Z] node_tests_1  |     at ClientRequest.once (events.js:340:11)
[2023-01-18T07:05:49.066Z] node_tests_1  |     at new ClientRequest (_http_client.js:174:10)
[2023-01-18T07:05:49.066Z] node_tests_1  |     at Object.request (http.js:39:10)
[2023-01-18T07:05:49.066Z] node_tests_1  |     at Object.request (/app/lib/instrumentation/http-shared.js:224:21)
[2023-01-18T07:05:49.066Z] node_tests_1  |     at /app/test/instrumentation/modules/hapi/shared.js:99:25
[2023-01-18T07:05:49.066Z] node_tests_1  |     at server.start.then (/app/test/instrumentation/modules/hapi/shared.js:606:15)
[2023-01-18T07:05:49.066Z] node_tests_1  |     at <anonymous>
[2023-01-18T07:05:49.066Z] node_tests_1  |     at process._tickCallback (internal/process/next_tick.js:189:7)
[2023-01-18T07:05:49.066Z] node_tests_1  | (node:136) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
[2023-01-18T07:05:49.066Z] node_tests_1  | (node:136) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
[2023-01-18T08:48:48.480Z] Sending interrupt signal to process

The 'http.request' instrumentation fix in #3090 uncovered a bug in our
tests for @hapi/hapi when with node v8 (a version before
`http.request(url, opts, cb)` was supported in node). This fixes that.

This also updates the .tav.yml section for '@hapi/hapi' to align with
logic in "_is_hapi_incompat.js" so the TAV tests don't waste time
installing a version of Hapi for which tests will be skipped anyway.

Refs: #3090
@trentm trentm self-assigned this Jan 18, 2023
@trentm
Copy link
Member Author

trentm commented Jan 18, 2023

run module tests for @hapi/hapi

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Jan 18, 2023
@trentm
Copy link
Member Author

trentm commented Jan 18, 2023

run module tests for @hapi/hapi

@ghost
Copy link

ghost commented Jan 18, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-18T18:21:56.717+0000

  • Duration: 35 min 23 sec

Test stats 🧪

Test Results
Failed 0
Passed 322188
Skipped 0
Total 322188

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

  • run benchmark tests : Run the benchmark test only.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@trentm trentm merged commit fcafc93 into main Jan 18, 2023
@trentm trentm deleted the trentm/hapi-node8-http.req branch January 18, 2023 19:04
PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this pull request Aug 20, 2024
The 'http.request' instrumentation fix in elastic#3090 uncovered a bug in our
tests for @hapi/hapi when with node v8 (a version before
`http.request(url, opts, cb)` was supported in node). This fixes that.

This also updates the .tav.yml section for '@hapi/hapi' to align with
logic in "_is_hapi_incompat.js" so the TAV tests don't waste time
installing a version of Hapi for which tests will be skipped anyway.

Refs: elastic#3090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant