diff --git a/.devcontainer.json b/.devcontainer.json index 10a610b4e7..c2812d7b6b 100644 --- a/.devcontainer.json +++ b/.devcontainer.json @@ -2,13 +2,12 @@ // For format details, see https://aka.ms/vscode-remote/devcontainer.json { "name": "docs.github.com", - "service": "container-doc", "settings": { "terminal.integrated.shell.linux": "/bin/bash", "cSpell.language": ",en" }, - // Install pre-requisites, and start to serve docs.github.com locally - "postCreateCommand": "npm install && npm start", + // Install pre-requisites and run a build to ensure we are ready to start serving docs.github.com locally (via `npm start`) + "postCreateCommand": "npm ci && npm run build", "forwardPorts": [4000], // Visual Studio Code extensions which help authoring for docs.github.com. "extensions": [ diff --git a/.github/workflows/autoupdate-branch.yml b/.github/workflows/autoupdate-branch.yml index 0e0729da46..c1e5b92b42 100644 --- a/.github/workflows/autoupdate-branch.yml +++ b/.github/workflows/autoupdate-branch.yml @@ -17,8 +17,6 @@ on: push: branches: - main - schedule: - - cron: '*/30 * * * *' # every 30 minutes jobs: autoupdate: diff --git a/.github/workflows/repo-sync.yml b/.github/workflows/repo-sync.yml index 1e08ecd3c9..cbc20bbc53 100644 --- a/.github/workflows/repo-sync.yml +++ b/.github/workflows/repo-sync.yml @@ -40,7 +40,7 @@ jobs: destination_branch: main pr_title: 'repo sync' pr_body: "This is an automated pull request to sync changes between the public and private repos.\n\n:robot: This pull request should be merged (not squashed) to preserve continuity across repos, so please let a bot do the merging!" - pr_label: autoupdate,automated-reposync-pr + pr_label: automerge,autoupdate,automated-reposync-pr github_token: ${{ secrets.OCTOMERGER_PAT_WITH_REPO_AND_WORKFLOW_SCOPE }} - name: Find pull request @@ -88,34 +88,6 @@ jobs: console.log(`Branch is already up-to-date`) } - - name: Enable GitHub auto-merge - if: ${{ steps.find-pull-request.outputs.number }} - uses: actions/github-script@626af12fe9a53dc2972b48385e7fe7dec79145c9 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const pull = await github.pulls.get({ - ...context.repo, - pull_number: parseInt(${{ steps.find-pull-request.outputs.number }}) - }) - - const pullNodeId = pull.data.node_id - console.log(`Pull request GraphQL Node ID: ${pullNodeId}`) - - const mutation = `mutation ($id: ID!) { - enablePullRequestAutoMerge(input: { - pullRequestId: $id, - mergeMethod: MERGE - }) { - clientMutationId - } - }` - const variables = { - id: pullNodeId - } - await github.graphql(mutation, variables) - console.log('Auto-merge enabled!') - - name: Send Slack notification if workflow fails uses: someimportantcompany/github-actions-slack-message@0b470c14b39da4260ed9e3f9a4f1298a74ccdefd if: failure() diff --git a/content/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository.md b/content/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository.md index 10a420e0db..56cc724cd5 100644 --- a/content/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository.md +++ b/content/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository.md @@ -12,9 +12,7 @@ versions: {% data reusables.discussions.about-discussions %} {% data reusables.discussions.about-categories-and-formats %} -Each category must have a unique name and emoji pairing, and can be accompanied by a detailed description stating its purpose. Categories help maintainers organize how conversations are filed and are customizable to help distinguish categories that are Q&A or more open-ended conversations.{% data reusables.discussions.repository-category-limit %} - -For more information, see "[About discussions](/discussions/collaborating-with-your-community-using-discussions/about-discussions#about-categories-and-formats-for-discussions)." +Each category must have a unique name and emoji pairing, and can be accompanied by a detailed description stating its purpose. Categories help maintainers organize how conversations are filed and are customizable to help distinguish categories that are Q&A or more open-ended conversations. {% data reusables.discussions.repository-category-limit %} For more information, see "[About discussions](/discussions/collaborating-with-your-community-using-discussions/about-discussions#about-categories-and-formats-for-discussions)." ### Default categories diff --git a/content/discussions/managing-discussions-for-your-community/managing-discussions-in-your-repository.md b/content/discussions/managing-discussions-for-your-community/managing-discussions-in-your-repository.md index 28251d0478..230201e4d9 100644 --- a/content/discussions/managing-discussions-for-your-community/managing-discussions-in-your-repository.md +++ b/content/discussions/managing-discussions-for-your-community/managing-discussions-in-your-repository.md @@ -24,7 +24,7 @@ To manage discussions in a repository, discussions must be enabled for the repos ### Changing the category for a discussion -You can categorize discussions to help community members find related discussions. For more information, see "[Managing categories for discussions in your repository](/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository)" article. +You can categorize discussions to help community members find related discussions. For more information, see "[Managing categories for discussions in your repository](/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository)." You can also move a discussion to a different category. diff --git a/content/discussions/quickstart.md b/content/discussions/quickstart.md index 252da3b6cc..4609b2fddb 100644 --- a/content/discussions/quickstart.md +++ b/content/discussions/quickstart.md @@ -16,7 +16,7 @@ Discussions give a space for more collaborative conversations by connecting and ### Enabling {% data variables.product.prodname_discussions %} on your repository -Repository owners and people with write access can enable {% data variables.product.prodname_discussions %} for a community on their public repositories. +Repository owners and people with write access can enable {% data variables.product.prodname_discussions %} for a community on their public and private repositories. When you first enable a {% data variables.product.prodname_discussions %}, you will be invited to configure a welcome post. @@ -49,13 +49,13 @@ Anyone with access to a repository can create a discussion. ### Organizing discussions into relevant categories -Repository owners and people with write access can create new categories to keep discussions organized. Collaborators participating and creating new discussions can group discussions into the most relevant existing categories. Discussions can also be recategorized after they are created. For more information, see "[Managing categories for discussions in your repository](/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository)" +Repository owners and people with write access can create new categories to keep discussions organized. Collaborators participating and creating new discussions can group discussions into the most relevant existing categories. Discussions can also be recategorized after they are created. For more information, see "[Managing categories for discussions in your repository](/discussions/managing-discussions-for-your-community/managing-categories-for-discussions-in-your-repository)." ### Promoting healthy conversations People with write permissions for a repository can help surface important conversations by pinning discussions, deleting discussions that are no longer useful or are damaging to the community, and transferring discussions to more relevant repositories owned by the organization. For more information, see "[Managing discussions in your repository](/discussions/managing-discussions-for-your-community/managing-discussions-in-your-repository)." -People with triage permissions for a repository can help moderate a project's discussions by marking comments as answers, locking discussions that are not longer useful or are damaging to the community, and converting issues to discussions when an idea is still in the early stages of development. For more information, see "[Moderating discussions](/discussions/managing-discussions-for-your-community/moderating-discussions)." +People with triage permissions for a repository can help moderate a project's discussions by marking comments as answers, locking discussions that are no longer useful or are damaging to the community, and converting issues to discussions when an idea is still in the early stages of development. For more information, see "[Moderating discussions](/discussions/managing-discussions-for-your-community/moderating-discussions)." ### Next steps diff --git a/data/reusables/gated-features/discussions.md b/data/reusables/gated-features/discussions.md index 5ada0a25ee..860427abe2 100644 --- a/data/reusables/gated-features/discussions.md +++ b/data/reusables/gated-features/discussions.md @@ -1 +1 @@ -{% data variables.product.prodname_discussions %} is available in beta for public repositories on {% data variables.product.prodname_dotcom_the_website %}. {% data reusables.gated-features.more-info-org-products %} +{% data variables.product.prodname_discussions %} is available in beta for public and private repositories on {% data variables.product.prodname_dotcom_the_website %}. {% data reusables.gated-features.more-info-org-products %} diff --git a/middleware/abort.js b/middleware/abort.js new file mode 100644 index 0000000000..a7351c29bf --- /dev/null +++ b/middleware/abort.js @@ -0,0 +1,15 @@ +module.exports = function (req, res, next) { + // If the client aborts the connection, send an error + req.once('aborted', () => { + // NOTE: Node.js will also automatically set `req.aborted = true` + + const abortError = new Error('Client closed request') + abortError.statusCode = 499 + abortError.code = 'ECONNRESET' + + // Pass the error to the Express error handler + return next(abortError) + }) + + return next() +} diff --git a/middleware/halt-on-dropped-connection.js b/middleware/halt-on-dropped-connection.js new file mode 100644 index 0000000000..9745723a84 --- /dev/null +++ b/middleware/halt-on-dropped-connection.js @@ -0,0 +1,18 @@ +function isConnectionDropped (req, res) { + // Have the flags been set for: + // - a global request timeout (via the express-timeout-handler middleware)? + // - an aborted request connection (via Node.js core's HTTP IncomingMessage)? + return Boolean(res.globalTimeout || req.aborted) +} + +function haltOnDroppedConnection (req, res, next) { + // Only proceed if the flag has not been set for the express-timeout-handler middleware + if (!isConnectionDropped(req, res)) { + return next() + } +} + +// Export this logic, too +haltOnDroppedConnection.isConnectionDropped = isConnectionDropped + +module.exports = haltOnDroppedConnection diff --git a/middleware/handle-errors.js b/middleware/handle-errors.js index 7e3757ae94..7fbb456ab9 100644 --- a/middleware/handle-errors.js +++ b/middleware/handle-errors.js @@ -6,7 +6,9 @@ const loadSiteData = require('../lib/site-data') function shouldLogException (error) { const IGNORED_ERRORS = [ // avoid sending CSRF token errors (from bad-actor POST requests) - 'EBADCSRFTOKEN' + 'EBADCSRFTOKEN', + // Client connected aborted + 'ECONNRESET' ] if (IGNORED_ERRORS.includes(error.code)) { @@ -26,8 +28,8 @@ async function logException (error, req) { } module.exports = async function handleError (error, req, res, next) { - // If the headers have already been sent... - if (res.headersSent) { + // If the headers have already been sent or the request was aborted... + if (res.headersSent || req.aborted) { // Report to Failbot await logException(error, req) diff --git a/middleware/index.js b/middleware/index.js index 6d39616432..77c99177fa 100644 --- a/middleware/index.js +++ b/middleware/index.js @@ -1,5 +1,6 @@ const express = require('express') const instrument = require('../lib/instrument-middleware') +const haltOnDroppedConnection = require('./halt-on-dropped-connection') const isDevelopment = process.env.NODE_ENV === 'development' @@ -11,6 +12,10 @@ const asyncMiddleware = fn => } module.exports = function (app) { + // *** Request connection management *** + app.use(require('./timeout')) + app.use(require('./abort')) + // *** Development tools *** app.use(require('morgan')('dev', { skip: (req, res) => !isDevelopment })) if (isDevelopment) app.use(require('./webpack')) @@ -60,6 +65,9 @@ module.exports = function (app) { app.use(instrument('./find-page')) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page app.use(instrument('./block-robots')) + // Check for a dropped connection before proceeding + app.use(haltOnDroppedConnection) + // *** Rendering, 2xx responses *** // I largely ordered these by use frequency app.use(instrument('./archived-enterprise-versions-assets')) // Must come before static/assets @@ -91,6 +99,9 @@ module.exports = function (app) { app.use(instrument('./loaderio-verification')) app.get('/_500', asyncMiddleware(instrument('./trigger-error'))) + // Check for a dropped connection before proceeding (again) + app.use(haltOnDroppedConnection) + // *** Preparation for render-page *** app.use(asyncMiddleware(instrument('./contextualizers/enterprise-release-notes'))) app.use(instrument('./contextualizers/graphql')) @@ -106,7 +117,12 @@ module.exports = function (app) { // *** Headers for pages only *** app.use(require('./set-fastly-cache-headers')) - // *** Rendering, must go last *** + // Check for a dropped connection before proceeding (again) + app.use(haltOnDroppedConnection) + + // *** Rendering, must go almost last *** app.get('/*', asyncMiddleware(instrument('./render-page'))) + + // *** Error handling, must go last *** app.use(require('./handle-errors')) } diff --git a/middleware/render-page.js b/middleware/render-page.js index 1ee5b3d161..0bb4e62774 100644 --- a/middleware/render-page.js +++ b/middleware/render-page.js @@ -6,6 +6,7 @@ const getMiniTocItems = require('../lib/get-mini-toc-items') const Page = require('../lib/page') const statsd = require('../lib/statsd') const RedisAccessor = require('../lib/redis-accessor') +const { isConnectionDropped } = require('./halt-on-dropped-connection') const { HEROKU_RELEASE_VERSION } = process.env const pageCacheDatabaseNumber = 1 @@ -28,31 +29,6 @@ function addCsrf (req, text) { module.exports = async function renderPage (req, res, next) { const page = req.context.page - // Remove any query string (?...) and/or fragment identifier (#...) - const { pathname, searchParams } = new URL(req.originalUrl, 'https://docs.github.com') - - for (const queryKey in req.query) { - if (!cacheableQueries.includes(queryKey)) { - searchParams.delete(queryKey) - } - } - const originalUrl = pathname + ([...searchParams].length > 0 ? `?${searchParams}` : '') - - // Serve from the cache if possible (skip during tests) - const isCacheable = !process.env.CI && process.env.NODE_ENV !== 'test' && req.method === 'GET' - - // Is the request for JSON debugging info? - const isRequestingJsonForDebugging = 'json' in req.query && process.env.NODE_ENV !== 'production' - - if (isCacheable && !isRequestingJsonForDebugging) { - const cachedHtml = await pageCache.get(originalUrl) - if (cachedHtml) { - console.log(`Serving from cached version of ${originalUrl}`) - statsd.increment('page.sent_from_cache') - return res.send(addCsrf(req, cachedHtml)) - } - } - // render a 404 page if (!page) { if (process.env.NODE_ENV !== 'test' && req.context.redirectNotFound) { @@ -70,15 +46,52 @@ module.exports = async function renderPage (req, res, next) { return res.status(200).end() } + // Remove any query string (?...) and/or fragment identifier (#...) + const { pathname, searchParams } = new URL(req.originalUrl, 'https://docs.github.com') + + for (const queryKey in req.query) { + if (!cacheableQueries.includes(queryKey)) { + searchParams.delete(queryKey) + } + } + const originalUrl = pathname + ([...searchParams].length > 0 ? `?${searchParams}` : '') + + // Serve from the cache if possible (skip during tests) + const isCacheable = !process.env.CI && process.env.NODE_ENV !== 'test' && req.method === 'GET' + + // Is the request for JSON debugging info? + const isRequestingJsonForDebugging = 'json' in req.query && process.env.NODE_ENV !== 'production' + + if (isCacheable && !isRequestingJsonForDebugging) { + // Stop processing if the connection was already dropped + if (isConnectionDropped(req, res)) return + + const cachedHtml = await pageCache.get(originalUrl) + if (cachedHtml) { + // Stop processing if the connection was already dropped + if (isConnectionDropped(req, res)) return + + console.log(`Serving from cached version of ${originalUrl}`) + statsd.increment('page.sent_from_cache') + return res.send(addCsrf(req, cachedHtml)) + } + } + // add page context const context = Object.assign({}, req.context, { page }) // collect URLs for variants of this page in all languages context.page.languageVariants = Page.getLanguageVariants(req.path) + // Stop processing if the connection was already dropped + if (isConnectionDropped(req, res)) return + // render page context.renderedPage = await page.render(context) + // Stop processing if the connection was already dropped + if (isConnectionDropped(req, res)) return + // get mini TOC items on articles if (page.showMiniToc) { context.miniTocItems = getMiniTocItems(context.renderedPage, page.miniTocMaxHeadingLevel) diff --git a/middleware/timeout.js b/middleware/timeout.js new file mode 100644 index 0000000000..b8190719ed --- /dev/null +++ b/middleware/timeout.js @@ -0,0 +1,27 @@ +const timeout = require('express-timeout-handler') + +// Heroku router requests timeout after 30 seconds. We should stop them earlier! +const maxRequestTimeout = parseInt(process.env.REQUEST_TIMEOUT, 10) || 25000 + +module.exports = timeout.handler({ + // Default timeout for all endpoints + // To override for a given router/endpoint, use `require('express-timeout-handler').set(...)` + timeout: maxRequestTimeout, + + // IMPORTANT: + // We cannot allow the middleware to disable the `res` object's methods like + // it does by default if we want to use `next` in the `onTimeout` handler! + disable: [], + + onTimeout: function (req, res, next) { + // Create a custom timeout error + const timeoutError = new Error('Request timed out') + timeoutError.statusCode = 503 + timeoutError.code = 'ETIMEDOUT' + + // Pass the error to our Express error handler for consolidated processing + return next(timeoutError) + } + + // Can also set an `onDelayedResponse` property IF AND ONLY IF you allow for disabling methods +}) diff --git a/package-lock.json b/package-lock.json index 847d8365d3..1f5f0dc58b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10914,6 +10914,11 @@ "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-5.1.3.tgz", "integrity": "sha512-TINcxve5510pXj4n9/1AMupkj3iWxl3JuZaWhCdYDlZeoCPqweGZrxbrlqTCFb1CT5wli7s8e2SH/Qz2c9GorA==" }, + "express-timeout-handler": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/express-timeout-handler/-/express-timeout-handler-2.2.0.tgz", + "integrity": "sha512-Ae9M32r3T+RmrY1MGwBvyVjnzVo1iD4s0YQHgvJqzPlT0d4qS84pr87LHyiUtnD7qDQLMYw6rl2fhsgH4TH8aQ==" + }, "ext": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/ext/-/ext-1.4.0.tgz", diff --git a/package.json b/package.json index 008ddf8da0..967d8253d9 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "dotenv": "^8.2.0", "express": "^4.17.1", "express-rate-limit": "^5.1.3", + "express-timeout-handler": "^2.2.0", "flat": "^5.0.0", "github-slugger": "^1.2.1", "got": "^9.6.0",