Repository: UmbrellaDocs/linkspector (js)

Issue:
[FEATURE REQ] Configuration Option for Redirects
**Is your feature request related to a problem? Please describe.**
With a configuration set to:

```yaml
aliveStatusCodes:
  - 200
```

Redirects (`301`) are also returned as passing the link checks.

**Describe the solution you'd like**
An option (`followRedirects`) that when set to `false`, would cause links that redirect to fail during the check. This would default to `true` when not configured so as not to be a breaking change for existing users.

**Describe alternatives you've considered**
The option could also be called `follow_redirects` to fit with the existing snake_case naming of options.

**Additional context**
If this proposal is accepted, I can provide a pull request that implements this.

Thanks for this project!

Pull request:
feat: add followRedirects option for configuration
## Description

AI generated.

This commit introduces a new configuration option, `followRedirects`, to control how HTTP redirects are handled by the link checker.

The `followRedirects` option can be set in the configuration file (e.g., `.linkspector.yml`):
- Type: boolean
- Default: `true`

Behavior:
- When `followRedirects` is `true` (default), links that redirect will be followed to their final destination. The status of the final destination will be reported.
- When `followRedirects` is `false`, if a link redirects, it will be reported as an 'error'. The original redirect status code (e.g., 301, 302) will be reported, and the link checker will not follow the redirect.

This feature allows you to identify and potentially fail checks for links that redirect, which can be useful for maintaining up-to-date and direct links.

Changes include:
- Added `followRedirects` to the configuration schema in `lib/validate-config.js`.
- Updated `lib/batch-check-links.js` to:
    - Respect the `followRedirects` option for both `fetch` and Puppeteer requests.
    - Report redirects as errors when `followRedirects` is `false`.
- Added a comprehensive test suite in `test/fixtures/redirects/` using Vitest, including a mock HTTP server to simulate various redirect scenarios.
- Updated `README.md` with documentation for the new option.

Fixes #121 

## Type of Change

- [x] New feature (non-breaking change which adds functionality)
- [x] This change requires a documentation update

## Checklist:

- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes

## Additional Information

Include any additional information about the pull request here.


Patch:
diff --git a/README.md b/README.md
index 3e2afdb..a1d3e27 100644
--- a/README.md
+++ b/README.md
@@ -113,6 +113,7 @@ Following are the available configuration options:
 | [`useGitIgnore`](#use-gitignore)                  | Indicates whether to use the rules defined in the `.gitignore` file to exclude files and directories. | No                                |
 | [`modifiedFilesOnly`](#check-modified-files-only) | Indicates whether to check only the files that have been modified in the last git commit.             | No                                |
 | [`httpHeaders`](#http-headers)                    | The list of URLs and their corresponding HTTP headers to be used during link checking.                | No                                |
+| [`followRedirects`](#follow-redirects)            | Controls how HTTP redirects (e.g., 301, 302) are handled.                                             | No                                |
 
 ### Files to Check
 
@@ -251,6 +252,29 @@ The `httpHeaders` option allows you to specify HTTP headers for specific URLs th
          Foo: Bar
    ```
 
+### Follow Redirects
+
+The `followRedirects` option controls how Linkspector handles HTTP redirects (e.g., status codes 301, 302).
+
+- **Type:** `boolean`
+- **Default:** `true`
+
+**Behavior:**
+
+- When `followRedirects: true` (default):
+  Linkspector will follow HTTP redirects to their final destination. The status of the link will be determined by the status code of this final destination. For example, if `http://example.com/old` redirects to `http://example.com/new` and `/new` returns a 200 OK, the original link `/old` will be reported as 'alive' (200), with a message indicating it was redirected.
+
+- When `followRedirects: false`:
+  Linkspector will _not_ follow HTTP redirects. If a link returns a redirect status code (e.g., 301, 302, 307, 308), it will be reported as an 'error'. The reported status code will be the original redirect status code (e.g., 301), and the error message will indicate that the link redirected but `followRedirects` was set to `false`.
+
+**Example:**
+
+To disable following redirects:
+
+```yaml
+followRedirects: false
+```
+
 ### Sample configuration
 
 ```yml
@@ -287,6 +311,7 @@ aliveStatusCodes:
   - 201
   - 204
 useGitIgnore: true
+followRedirects: false # Example of including it in a full config
 ```
 
 ## Sample output
diff --git a/lib/batch-check-links.js b/lib/batch-check-links.js
index f65e2da..9f9fc23 100644
--- a/lib/batch-check-links.js
+++ b/lib/batch-check-links.js
@@ -22,7 +22,13 @@ function createLinkStatus(link, status, statusCode, errorMessage = null) {
   }
 }
 
-async function processLink(link, page, aliveStatusCodes, httpHeaders) {
+async function processLink(
+  link,
+  page,
+  aliveStatusCodes,
+  httpHeaders,
+  followRedirects
+) {
   let status = null
   let statusCode = null
   let errorMessage = null
@@ -35,11 +41,20 @@ async function processLink(link, page, aliveStatusCodes, httpHeaders) {
         )?.headers || {}
 
       const response = await page.goto(link.url, {
-        waitUntil: 'load',
+        waitUntil: 'load', // Puppeteer follows redirects by default.
         headers,
       })
       statusCode = response.status()
-      if (aliveStatusCodes && aliveStatusCodes.includes(statusCode)) {
+      const redirectChain = response.request().redirectChain()
+
+      if (!followRedirects && redirectChain.length > 0) {
+        // If followRedirects is false and there was a redirect
+        status = 'error'
+        const originalStatusCode = redirectChain[0].response().status()
+        errorMessage = `Link redirected (from ${redirectChain[0].url()} status: ${originalStatusCode} to ${response.url()}), but followRedirects is set to false.`
+        // We might want to use the original redirect status code if available and makes sense
+        statusCode = originalStatusCode !== 0 ? originalStatusCode : statusCode
+      } else if (aliveStatusCodes && aliveStatusCodes.includes(statusCode)) {
         status = 'assumed alive'
       } else {
         status = response.ok() ? 'alive' : 'error'
@@ -59,6 +74,7 @@ async function checkHyperlinks(nodes, options = {}, filePath) {
     retryCount = 3,
     aliveStatusCodes,
     httpHeaders = [],
+    followRedirects = true, // Default to true if not provided
   } = options
   const linkStatusList = []
   const tempArray = []
@@ -74,16 +90,36 @@ async function checkHyperlinks(nodes, options = {}, filePath) {
   for (let link of filteredNodes) {
     try {
       if (isUrl(link.url)) {
-        const response = await fetch(link.url, {
+        const fetchOptions = {
           method: 'HEAD',
-          redirect: 'follow',
-        })
-        let message = response.redirected
-          ? `redirected to ${response.url}`
-          : null
+          redirect: followRedirects ? 'follow' : 'manual',
+        }
+        const response = await fetch(link.url, fetchOptions)
         const statusCode = response.status
+        let message = null
+
+        // Handle manual redirect: if followRedirects is false and a redirect occurs
+        if (
+          !followRedirects &&
+          (response.type === 'opaqueredirect' ||
+            [301, 302, 307, 308].includes(statusCode))
+        ) {
+          const redirectedTo = response.headers.get('location')
+          const errorMessage = `Link redirected${redirectedTo ? ' to ' + redirectedTo : ''}, but followRedirects is set to false.`
+          const linkStatus = createLinkStatus(
+            link,
+            'error',
+            statusCode === 0 && response.type === 'opaqueredirect'
+              ? 302
+              : statusCode, // Use 302 for opaque, else actual
+            errorMessage
+          )
+          linkStatusList.push(linkStatus)
+          continue
+        }
 
         if (response.ok) {
+          message = response.redirected ? `redirected to ${response.url}` : null
           const linkStatus = createLinkStatus(
             link,
             'alive',
@@ -97,6 +133,7 @@ async function checkHyperlinks(nodes, options = {}, filePath) {
           linkStatusList.push(linkStatus)
           continue
         } else {
+          // If not ok, and not an explicit redirect handled above, or not in aliveStatusCodes
           tempArray.push(link)
         }
       } else {
@@ -167,7 +204,8 @@ async function checkHyperlinks(nodes, options = {}, filePath) {
               link,
               page,
               aliveStatusCodes,
-              httpHeaders
+              httpHeaders,
+              followRedirects // Pass followRedirects here
             )
             break
           } catch (error) {
diff --git a/lib/validate-config.js b/lib/validate-config.js
index 6e89685..ba18774 100644
--- a/lib/validate-config.js
+++ b/lib/validate-config.js
@@ -46,6 +46,7 @@ async function validateConfig(config) {
       //showErrorsOnly: Joi.boolean(),
       useGitIgnore: Joi.boolean(),
       modifiedFilesOnly: Joi.boolean(),
+      followRedirects: Joi.boolean().default(true),
     }).or('files', 'dirs')
 
     // Validate the config against the schema


Test patch:
diff --git a/index.test.js b/index.test.js
index 29e930b..e8fef66 100644
--- a/index.test.js
+++ b/index.test.js
@@ -34,7 +34,7 @@ test('linkspector should check top-level relative links in Markdown file', async
   }
 
   expect(hasErrorLinks).toBe(false)
-  expect(results.length).toBe(22)
+  expect(results.length).toBe(23)
 })
 
 test('linkspector should track statistics correctly when stats option is enabled', async () => {
@@ -87,7 +87,7 @@ test('linkspector should track statistics correctly when stats option is enabled
 
   // Verify statistics are being tracked correctly
   expect(stats.filesChecked).toBeGreaterThan(0)
-  expect(stats.totalLinks).toBe(22)
+  expect(stats.totalLinks).toBe(23)
   expect(stats.totalLinks).toBe(
     stats.httpLinks +
       stats.fileLinks +
diff --git a/test/fixtures/redirects/config-redirects-false.yml b/test/fixtures/redirects/config-redirects-false.yml
new file mode 100644
index 0000000..17f02e4
--- /dev/null
+++ b/test/fixtures/redirects/config-redirects-false.yml
@@ -0,0 +1,3 @@
+files:
+  - test/fixtures/redirects/redirects.md
+followRedirects: false
diff --git a/test/fixtures/redirects/config-redirects-true.yml b/test/fixtures/redirects/config-redirects-true.yml
new file mode 100644
index 0000000..445d419
--- /dev/null
+++ b/test/fixtures/redirects/config-redirects-true.yml
@@ -0,0 +1,4 @@
+files:
+  - test/fixtures/redirects/redirects.md
+# followRedirects is true by default, so we can omit it or set it explicitly.
+# For clarity in testing, we can omit it to test the default behavior.
diff --git a/test/fixtures/redirects/redirects.md b/test/fixtures/redirects/redirects.md
new file mode 100644
index 0000000..2facdb6
--- /dev/null
+++ b/test/fixtures/redirects/redirects.md
@@ -0,0 +1,25 @@
+# Redirect Test Links
+
+This file contains links for testing the followRedirects feature.
+
+## Scenario 1 & 2: Redirecting Link
+
+[Permanent Redirect (301)](http://localhost:3000/redirect-permanent)
+[Temporary Redirect (302)](http://localhost:3000/redirect-temporary)
+
+## Scenario 3 & 4: Non-Redirecting Link
+
+[Direct OK (200)](http://localhost:3000/ok)
+
+## Scenario 5: Error Link
+
+[Not Found (404)](http://localhost:3000/not-found)
+
+## Scenario: Redirect to external allowed
+
+[External Redirect (301)](http://localhost:3000/redirect-external)
+[Final External Destination](https://example.com)
+
+## Scenario: Redirect loop
+
+[Redirect Loop](http://localhost:3000/redirect-loop1)
diff --git a/test/fixtures/redirects/redirects.test.js b/test/fixtures/redirects/redirects.test.js
new file mode 100644
index 0000000..7d036b9
--- /dev/null
+++ b/test/fixtures/redirects/redirects.test.js
@@ -0,0 +1,253 @@
+import { describe, it, expect, beforeAll, afterAll } from 'vitest'
+import { linkspector } from '../../../linkspector.js' // Import from root linkspector.js
+import path from 'path'
+import http from 'http'
+import { fileURLToPath } from 'url'
+
+const __filename = fileURLToPath(import.meta.url)
+const __dirname = path.dirname(__filename)
+
+const fixturesDir = path.join(__dirname)
+// const markdownFile = path.join(fixturesDir, 'redirects.md') // No longer directly passed
+const configFileFollowFalse = path.join(
+  fixturesDir,
+  'config-redirects-false.yml'
+)
+const configFileFollowTrue = path.join(fixturesDir, 'config-redirects-true.yml')
+
+let server
+
+const PORT = 3000
+const HOST = 'localhost'
+
+const serverHandler = (req, res) => {
+  if (req.url === '/redirect-permanent') {
+    res.writeHead(301, { Location: `http://${HOST}:${PORT}/final-destination` })
+    res.end()
+  } else if (req.url === '/redirect-temporary') {
+    res.writeHead(302, { Location: `http://${HOST}:${PORT}/final-destination` })
+    res.end()
+  } else if (req.url === '/final-destination') {
+    res.writeHead(200, { 'Content-Type': 'text/plain' })
+    res.end('Final Destination Reached')
+  } else if (req.url === '/ok') {
+    res.writeHead(200, { 'Content-Type': 'text/plain' })
+    res.end('OK')
+  } else if (req.url === '/redirect-external') {
+    res.writeHead(301, { Location: 'https://example.com' })
+    res.end()
+  } else if (req.url === '/redirect-loop1') {
+    res.writeHead(302, { Location: `http://${HOST}:${PORT}/redirect-loop2` })
+    res.end()
+  } else if (req.url === '/redirect-loop2') {
+    res.writeHead(302, { Location: `http://${HOST}:${PORT}/redirect-loop1` })
+    res.end()
+  } else {
+    res.writeHead(404, { 'Content-Type': 'text/plain' })
+    res.end('Not Found')
+  }
+}
+
+describe('followRedirects feature', () => {
+  beforeAll(async () => {
+    server = http.createServer(serverHandler)
+    await new Promise((resolve) => server.listen(PORT, HOST, resolve))
+  })
+
+  afterAll(async () => {
+    await new Promise((resolve) => server.close(resolve))
+  })
+
+  // Scenario 1: followRedirects: true (default) - Permanent Redirect (301)
+  it('should report a permanent redirecting link as alive (200) when followRedirects is true (default)', async () => {
+    const resultsAsync = linkspector(configFileFollowTrue, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const redirectLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-permanent`
+    )
+    expect(redirectLink.status).toBe('alive')
+    expect(redirectLink.status_code).toBe(200) // Final destination
+    expect(redirectLink.error_message).toContain('redirected to')
+  }, 10000) // Increased timeout to 10 seconds
+
+  // Scenario 1 (bis): followRedirects: true (default) - Temporary Redirect (302)
+  it('should report a temporary redirecting link as alive (200) when followRedirects is true (default)', async () => {
+    const resultsAsync = linkspector(configFileFollowTrue, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const redirectLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-temporary`
+    )
+    expect(redirectLink.status).toBe('alive')
+    expect(redirectLink.status_code).toBe(200) // Final destination
+    expect(redirectLink.error_message).toContain('redirected to')
+  }, 10000) // Increased timeout
+
+  // Scenario 2: followRedirects: false - Permanent Redirect (301)
+  it('should report a permanent redirecting link as error (301) when followRedirects is false', async () => {
+    const resultsAsync = linkspector(configFileFollowFalse, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const redirectLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-permanent`
+    )
+    expect(redirectLink.status).toBe('error')
+    expect(redirectLink.status_code).toBe(301)
+    expect(redirectLink.error_message).toMatch(
+      /redirected.*followRedirects is set to false/i
+    )
+  })
+
+  // Scenario 2 (bis): followRedirects: false - Temporary Redirect (302)
+  it('should report a temporary redirecting link as error (302) when followRedirects is false', async () => {
+    const resultsAsync = linkspector(configFileFollowFalse, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const redirectLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-temporary`
+    )
+    expect(redirectLink.status).toBe('error')
+    expect(redirectLink.status_code).toBe(302)
+    expect(redirectLink.error_message).toMatch(
+      /redirected.*followRedirects is set to false/i
+    )
+  })
+
+  // Scenario 3: Non-redirecting link with followRedirects: false
+  it('should report a non-redirecting link as alive (200) when followRedirects is false', async () => {
+    const resultsAsync = linkspector(configFileFollowFalse, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const okLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/ok`
+    )
+    expect(okLink.status).toBe('alive')
+    expect(okLink.status_code).toBe(200)
+  })
+
+  // Scenario 4: Non-redirecting link with followRedirects: true (default)
+  it('should report a non-redirecting link as alive (200) when followRedirects is true (default)', async () => {
+    const resultsAsync = linkspector(configFileFollowTrue, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const okLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/ok`
+    )
+    expect(okLink.status).toBe('alive')
+    expect(okLink.status_code).toBe(200)
+  })
+
+  // Scenario 5: Link that results in an actual error (404) with followRedirects: false
+  it('should report a 404 link as error (404) when followRedirects is false', async () => {
+    const resultsAsync = linkspector(configFileFollowFalse, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const notFoundLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/not-found`
+    )
+    expect(notFoundLink.status).toBe('error')
+    expect(notFoundLink.status_code).toBe(404)
+  })
+
+  // Scenario: Link that results in an actual error (404) with followRedirects: true (default)
+  it('should report a 404 link as error (404) when followRedirects is true (default)', async () => {
+    const resultsAsync = linkspector(configFileFollowTrue, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const notFoundLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/not-found`
+    )
+    expect(notFoundLink.status).toBe('error')
+    expect(notFoundLink.status_code).toBe(404)
+  })
+
+  // Scenario: External redirect allowed when followRedirects is true
+  it('should report an external redirecting link as alive (200 from example.com) when followRedirects is true', async () => {
+    const resultsAsync = linkspector(configFileFollowTrue, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const externalRedirectLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-external`
+    )
+    expect(externalRedirectLink.status).toBe('alive')
+    // Note: status code might be from the final destination (example.com) if HEAD request works,
+    // or could be tricky if example.com blocks HEAD. Puppeteer fallback should handle it.
+    // For now, checking for 'alive' is the primary goal.
+    // expect(externalRedirectLink.status_code).toBe(200) // This can be flaky with external sites
+    expect(externalRedirectLink.error_message).toContain(
+      'redirected to https://example.com'
+    )
+  }, 10000) // Increased timeout
+
+  // Scenario: External redirect disallowed when followRedirects is false
+  it('should report an external redirecting link as error (301) when followRedirects is false', async () => {
+    const resultsAsync = linkspector(configFileFollowFalse, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const externalRedirectLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-external`
+    )
+    expect(externalRedirectLink.status).toBe('error')
+    expect(externalRedirectLink.status_code).toBe(301)
+    expect(externalRedirectLink.error_message).toMatch(
+      /redirected to https:\/\/example.com, but followRedirects is set to false/i
+    )
+  })
+
+  // Scenario: Redirect loop when followRedirects is true (Puppeteer should eventually error out)
+  it('should report a redirect loop as error when followRedirects is true', async () => {
+    // This test might take a bit longer due to Puppeteer's retries for loops or timeouts
+    const resultsAsync = linkspector(configFileFollowTrue, {
+      aliveStatusCodes: [200],
+    }) // Ensure only 200 is "assumed alive"
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const loopLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-loop1`
+    )
+    expect(loopLink.status).toBe('error')
+    // The error message might vary depending on how Puppeteer handles max redirects
+    // e.g., "net::ERR_TOO_MANY_REDIRECTS" or similar
+    expect(loopLink.error_message).toBeDefined()
+  }, 20000) // Timeout already increased, keeping it
+
+  // Scenario: Redirect loop when followRedirects is false
+  it('should report a redirect loop as error (first redirect status) when followRedirects is false', async () => {
+    const resultsAsync = linkspector(configFileFollowFalse, {})
+    const collectedResults = []
+    for await (const item of resultsAsync) {
+      collectedResults.push(...item.result)
+    }
+    const loopLink = collectedResults.find(
+      (r) => r.link === `http://${HOST}:${PORT}/redirect-loop1`
+    )
+    expect(loopLink.status).toBe('error')
+    expect(loopLink.status_code).toBe(302) // The first redirect in the loop
+    expect(loopLink.error_message).toMatch(
+      /redirected.*followRedirects is set to false/i
+    )
+  })
+})