Bootstrap embedded docs#677
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the short keep-alive troubleshooting URL in Node client warning logs with a permanent GitHub docs link, and adds new embedded documentation pages describing keep-alive ECONNRESET/socket hang-up scenarios.
Changes:
- Update keep-alive mismatch warning link from
c.houseshort URL todocs/howto/keep_alive_timeout.md. - Add embedded docs pages for keep-alive timeout troubleshooting and general socket hang-up/ECONNRESET guidance.
- Update integration test and changelog example to match the new link.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/socket_pool.ts | Updates warning log message to point at the new embedded keep-alive timeout doc. |
| packages/client-node/tests/integration/node_keep_alive_header.test.ts | Adjusts log matching in tests to the new doc link. |
| docs/socket_hang_up_econnreset.md | Adds general troubleshooting guide for socket hang up / ECONNRESET. |
| docs/howto/keep_alive_timeout.md | Adds focused guide explaining keep-alive timeout mismatch and how to debug/fix it. |
| CHANGELOG.md | Updates example warning log message URL. |
| AGENTS.md | Adds guidance for agents on logging guard checks and linking to embedded docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Node client keep-alive troubleshooting guidance by replacing the shortlink previously used in keep-alive/ECONNRESET warning logs with repository-hosted documentation, and adds new embedded docs pages to support these user-facing suggestions.
Changes:
- Updated keep-alive warning log message (and its tests + changelog example) to link to
docs/howto/keep_alive_timeout.md. - Added embedded troubleshooting docs under
docs/(including a dedicated keep-alive timeout how-to). - Updated
AGENTS.mdrecommendations to codify logging + “link to docs page” guidance.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/socket_pool.ts | Updates WARN log message URL for keep-alive timeout mismatch guidance. |
| packages/client-node/tests/integration/node_keep_alive_header.test.ts | Adjusts integration test assertions to match the updated URL; minor whitespace cleanup. |
| docs/socket_hang_up_econnreset.md | Adds a general troubleshooting page for socket hang up / ECONNRESET. |
| docs/howto/keep_alive_timeout.md | Adds a focused how-to explaining keep-alive idle TTL vs server timeout behavior. |
| CHANGELOG.md | Updates example log output to reflect the new documentation URL. |
| AGENTS.md | Adds explicit guidance for eager log-level guards and linking logs to embedded docs pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the shortlink used in keep-alive/ECONNRESET warning logs with a GitHub-hosted docs link, and bootstraps new embedded troubleshooting documentation pages in-repo.
Changes:
- Update keep-alive warning log URL (and related tests / changelog example) to point at
docs/howto/keep_alive_timeout.md. - Add new troubleshooting docs pages for keep-alive timeout mismatch and generic
socket hang up/ECONNRESET. - Update
SocketPoolto use a runtimehttpimport and aninstanceof Http.Agentguard before iteratingfreeSockets.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/socket_pool.ts | Updates agent guard logic and warning log link. |
| packages/client-node/tests/integration/node_keep_alive_header.test.ts | Adjusts test expectations to match the new docs URL. |
| docs/socket_hang_up_econnreset.md | Adds a new troubleshooting guide for socket hang up / ECONNRESET. |
| docs/howto/keep_alive_timeout.md | Adds a focused keep-alive timeout mismatch guide referenced by logs. |
| CHANGELOG.md | Updates the example warning log message link. |
| AGENTS.md | Adds contributor/agent guidance for logging and doc-linking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the existing keep-alive troubleshooting shortlink with in-repo documentation, adds new docs pages for ECONNRESET/keep-alive timeout guidance, and adjusts keep-alive socket cleanup guarding in the Node client.
Changes:
- Update the Node keep-alive ECONNRESET warning to link to
docs/howto/keep_alive_timeout.mdand adjust related tests/CHANGELOG example. - Add new troubleshooting documentation under
docs/(including a newdocs/howtopage). - Tighten the eager stale-socket cleanup guard to run only for
http.Agentinstances (based on aninstanceofcheck).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/socket_pool.ts | Updates http import and guards eager stale-socket cleanup; updates warning link to new docs page. |
| packages/client-node/tests/integration/node_keep_alive_header.test.ts | Updates assertions to match the new docs link emitted in logs. |
| docs/socket_hang_up_econnreset.md | Adds a new troubleshooting guide for socket hang up / ECONNRESET. |
| docs/howto/keep_alive_timeout.md | Adds a focused how-to page explaining keep-alive timeout vs idle_socket_ttl. |
| CHANGELOG.md | Updates the example warning log message URL to the new docs page. |
| AGENTS.md | Adds explicit guidance for agents about log-level guards and linking to docs/ pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR “bootstraps embedded docs” by adding dedicated markdown troubleshooting pages and updating the Node client’s keep-alive warning log to link to the new documentation, with corresponding test and changelog updates.
Changes:
- Update the keep-alive ECONNRESET warning link to point to a new
docs/howto/keep_alive_timeout.mdpage. - Add new troubleshooting documentation pages under
docs/. - Tighten eager stale-socket cleanup to run only when using the built-in Node.js
http.Agent(viainstanceof), and adjust integration tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/socket_pool.ts | Updates HTTP import to allow instanceof check; restricts eager stale-socket cleanup to http.Agent; updates warning link to new embedded docs. |
| packages/client-node/tests/integration/node_keep_alive_header.test.ts | Updates log matching regex to reflect new docs URL and removes an extra blank line. |
| docs/socket_hang_up_econnreset.md | Adds a general troubleshooting guide for socket hang up / ECONNRESET. |
| docs/howto/keep_alive_timeout.md | Adds a focused how-to page explaining keep-alive timeout vs idle socket TTL and mitigation steps. |
| CHANGELOG.md | Updates the example warning message to the new docs URL. |
| AGENTS.md | Adds explicit guidance for eager log-level guards and linking user-suggestion logs to dedicated docs pages; removes the prior “ongoing refactoring” notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 task
peter-leonov-ch
added a commit
that referenced
this pull request
May 9, 2026
`AGENTS.md` had drifted from reality after PRs #662–#712: it claimed `examples/web` has no `performance/` folder, pointed at a placeholder docs path, and said nothing about the `skills/` directory or the `.claude/skills/setup/` skill that replaced `copilot-setup-steps.yml`. ## Summary - **Examples › `performance/`** — drop the "no `performance/` folder under `examples/web`" claim; #701 added one for browser-safe streaming. - **Log-message docs guidance** — point at the real `docs/` layout from #677 (`docs/socket_hang_up_econnreset.md`, `docs/howto/`) instead of a fictional `docs/example-log-message.md`. The example URL in the snippet now also resolves to a real page (`docs/socket_hang_up_econnreset.md`). - **New `Skills` section** — documents the repo-root `skills/` directory (#681, #682, #702): `client-node`'s `prepack` ships it via `@clickhouse/client`, the `agents.skills` field of `packages/client-node/package.json` declares which skills are discoverable, and the `Skills E2E` workflow (`.github/workflows/e2e-skills.yml`, backed by `tests/e2e/skills/check.js`) asserts the packaged tarball contains the declared skills. Routes contributors to `.claude/skills/setup/SKILL.md` (#705) instead of re-introducing `copilot-setup-steps.yml`. - **`agents.skills` manifest** — adds `clickhouse-js-node-coding` alongside `clickhouse-js-node-troubleshooting` so both shipped skills are discoverable to downstream tooling. `tests/e2e/skills/check.js` is extended to verify both skills land in the packaged `@clickhouse/client` tarball. - **New `Embedded docs` section** — surfaces `docs/` as the preferred home for pages linked from log messages. - **CHANGELOG guidance** — aligns with the PR template: entry goes in the PR description and is folded into `CHANGELOG.md` at release time. Documentation refresh plus a small manifest/test change to make the second shipped skill discoverable. ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A short description of the changes with a link to an open issue.
Checklist
Delete items not relevant to your PR: