Repository: tcort/markdown-link-check

Issue:
Error: Callback was already called. du to race condition
### Have you checked that the bug is not already fixed?

Yes

### Summary

markdown-link-check crashes somewhat randomly with `Error: Callback was already called.`

### Affected version

3.13.7, commit b88c7e6e9cea93a54d7ce4d4f5f56b61f6eebc0b

### Steps to reproduce the bug

  
It seems it check the same link twice, calling a callback create by `onlyOnce` twice, causing a crash  



Minimal example below:
For privacy reason, I cannot disclose the exact links, but they return a 200 statusCode.  
I was not able to find a public link pairs that triggered the crash.

dir1/index.md
```markdown
<https://link1.com>
```

dir2/index.md:
```markdown
<https://link2.com>
```

```
$ docker run -it -v $HOME/dummy_doc/:/tmp:ro $(docker build -q .) --verbose /tmp/

FILE: /tmp/dir1/index.md
ERROR: XXX null LinkCheckResult {
  link: 'https://link1.com',
  statusCode: 200,
  err: null,
  status: 'alive'
}
  [✓] https://link1.com → Status: 200

  1 links checked.

FILE: /tmp/dir2/index.md
ERROR: XXX null LinkCheckResult {
  link: 'https://link1.com',
  statusCode: 200,
  err: null,
  status: 'alive'
}
/src/node_modules/async/dist/async.js:325
            if (fn === null) throw new Error("Callback was already called.");
                             ^

Error: Callback was already called.
    at /src/node_modules/async/dist/async.js:325:36
    at /src/node_modules/async/dist/async.js:249:17
    at /src/index.js:214:13
    at /src/node_modules/link-check/lib/proto/http.js:119:21
    at done (/src/node_modules/needle/lib/needle.js:474:14)
    at PassThrough.<anonymous> (/src/node_modules/needle/lib/needle.js:734:9)
    at PassThrough.emit (node:events:519:35)
    at endReadableNT (node:internal/streams/readable:1701:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:90:21)
```

The error log comes from this line I added:
```diff
diff --git a/index.js b/index.js
index 167e9d4..5687ec2 100644
--- a/index.js
+++ b/index.js
@@ -209,6 +209,8 @@ module.exports = function markdownLinkCheck(markdown, opts, callback) {
                 result.status = 'error'; // custom status for errored links
             }
 
+            console.error("ERROR: XXX %s", err, result);
+
             callback(null, result);
         });
     }, callback);
```

Note that markdown-link-check believe the `dir2/index.md` contain a link to `link1.com`, which is false.  
In other runs, markdown-link-check believed `dir2/index.md` contained the two links  

To me it looks like a race condition

The problem only appear for some links

It is probably linked to: https://github.com/tcort/markdown-link-check/issues/222

### Expected behavior

It should not crash and don't mix up links

### Actual behavior

-

### Additional context

_No response_

### Can you fix it?

Yes

Hints from issue comments:
@github-actions: This issue has been marked as stale because it has been open 60 days with no activity. It will be closed in 90 days unless the stale label is removed or someone adds a comment.@cristiklein: I believe I just hit this with version 3.14.1:

```
Markdown Link Check.....................................................................Failed
- hook id: markdown-link-check
- exit code: 1

/home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/node_modules/async/dist/async.js:325
            if (fn === null) throw new Error("Callback was already called.");
                             ^

Error: Callback was already called.
    at /home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/node_modules/async/dist/async.js:325:36
    at /home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/node_modules/async/dist/async.js:249:17
    at /home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/index.js:212:13
    at /home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/node_modules/link-check/lib/proto/http.js:119:21
    at done (/home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/node_modules/needle/lib/needle.js:474:14)
    at PassThrough.<anonymous> (/home/cklein/.cache/pre-commit/repon5a1wdl_/node_env-system/lib/node_modules/markdown-link-check/node_modules/needle/lib/needle.js:734:9)
    at PassThrough.emit (node:events:531:35)
    at endReadableNT (node:internal/streams/readable:1698:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:90:21)

Node.js v22.21.0
```@BaseMax: > I believe I just hit this with version 3.14.1

Thanks for reporting this. Do you have the ability to fix it by submitting a PR? We will review and merge it soon.
Best regards.@cristiklein: Hi. Thanks for the quick answer. I haven't yet managed to understand the issue. However, I managed to come up with a minimum viable example:

```markdown
<https://imy.se>                                                          
<https://www.google.com/> 
<https://www.imy.se/>    
```

The first URL returns 302 FOUND and `Location: https://www.imy.se/`. That seems to be key to triggering this error.

I tried to add some `console.log()` in `index.js`, but don't quite understand which callback is called twice and why.

Any helpful pointers?@cristiklein: Thanks @BaseMax for the encouragement! I believe I found it: https://github.com/tcort/markdown-link-check/pull/540@BaseMax: Close - from https://github.com/tcort/markdown-link-check/pull/540@BaseMax: Please double check it and in case of any problem re-open it again.
@pixelshot91 

Thanks.@pixelshot91: I retried with the version 3.14.2, and it does not crash anymore.

Instead it prints this error and goes on.

```
Trace: linkCheck called us back more than once for <REDACTED_LINK>. This is likely due to a server answering HEAD with 302 and a body, against the HTTP spec. Ignoring.
    at /src/index.js:205:25
    at /src/node_modules/link-check/lib/proto/http.js:119:21
    at done (/src/node_modules/needle/lib/needle.js:474:14)
    at PassThrough.<anonymous> (/src/node_modules/needle/lib/needle.js:734:9)
    at PassThrough.emit (node:events:520:35)
    at endReadableNT (node:internal/streams/readable:1701:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:89:21)
```

The query did answer a 302, but I don't think it return a body.
The error only happen if two files contain the same REDACTED_LINK to a 302.

I tried with this link: `https://mock.httpstatus.io/302`, but it does not trigger the error.

I am sorry but I cannot provide the REDACTED_LINK.

Anyway, it does not crash anymore.
Thank you 👍 @cristiklein: @pixelshot91 The message you get is exactly what the PR was meant to achieve. Inside `markdown-link-check` a library called `linkCheck` is calling back twice, which is odd and I felt like this should not be hidden.

In the meanwhile, I found that the root cause is a [bug in a library called needle](https://github.com/tomas/needle/pull/444), which is used by `linkcheck` which is used by `markdown-link-check`. Once the fix makes it all the way to `markdown-link-check` the "Trace: linkCheck called us back more than once for" message should also disappear.

Solution patch:
diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index 0000000..3eda792
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,7 @@
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+indent_style = space
+indent_size = 4
diff --git a/index.js b/index.js
index 167e9d4..032903f 100644
--- a/index.js
+++ b/index.js
@@ -199,7 +199,14 @@ module.exports = function markdownLinkCheck(markdown, opts, callback) {
             return;
         }
 
+        let numCalls = 0;
         linkCheck(link, opts, function (err, result) {
+            if (numCalls > 0) {
+                console.trace(`linkCheck called us back more than once for ${link}. This is likely due to a server answering HEAD with 302 and a body, against the HTTP spec. Ignoring.`);
+                return;
+            }
+            numCalls += 1;
+
             if (opts.showProgressBar) {
                 bar.tick();
             }



Test patch:
diff --git a/test/markdown-link-check.test.js b/test/markdown-link-check.test.js
index a2a72c1..10572bf 100644
--- a/test/markdown-link-check.test.js
+++ b/test/markdown-link-check.test.js
@@ -53,6 +53,22 @@ describe('markdown-link-check', function () {
             res.json({foo:'bar'});
         });
 
+        app.use('/redirect-with-body-in-head', function (req, res) {
+            /* Don't judge me. I found at least one server on the
+             * Internet doing this and it triggered a bug. */
+            const body = 'Let me send you a body, although you only asked for a HEAD.';
+
+            const headers =
+                'HTTP/1.1 302 Found\r\n' +
+                'Connection: close\r\n' +
+                'Location: /foo/bar\r\n' +
+                `Content-Length: ${Buffer.byteLength(body)}\r\n` +
+                '\r\n';
+
+            res.socket.write(headers + body);
+            res.socket.destroy();
+        });
+
         app.get('/basic-auth', function (req, res) {
             if (req.headers["authorization"] === "Basic Zm9vOmJhcg==") {
                 res.sendStatus(200);
@@ -110,6 +126,9 @@ describe('markdown-link-check', function () {
             expect(results).to.be.an('array');
 
             const expected = [
+                // redirect-with-body-in-head
+                { statusCode: 200, status: 'alive' },
+
                 // redirect-loop
                 { statusCode:   0, status:  'dead' },
 
diff --git a/test/sample.md b/test/sample.md
index 830803f..42f13e8 100644
--- a/test/sample.md
+++ b/test/sample.md
@@ -2,6 +2,7 @@
 
 This is a test file:
 
+* [redirect-with-body-in-head](%%BASE_URL%%/redirect-with-body-in-head) (alive)
 * [redirect-loop](%%BASE_URL%%/loop) (dead)
 * [valid](%%BASE_URL%%/foo/bar) (alive)
 * [invalid](%%BASE_URL%%/foo/dead) (dead)