fix(std/node): Stop callbacks being called twice when callback throws error (#8867)

1 files changed, 53 insertions, 0 deletions

diff --git a/std/node/README.md b/std/node/README.md
index 6b0d9258e..5f0d8511a 100644
--- a/std/node/README.md
+++ b/std/node/README.md
@@ -82,3 +82,56 @@ const cjsModule = require("./my_mod");
 // Visits node_modules.
 const leftPad = require("left-pad");
 ```
+
+## Contributing
+
+When converting from promise-based to callback-based APIs, the most obvious way
+is like this:
+
+```ts
+promise.then((value) => callback(null, value)).catch(callback);
+```
+
+This has a subtle bug - if the callback throws an error, the catch statement
+will also catch _that_ error, and the callback will be called twice. The correct
+way to do it is like this:
+
+```ts
+promise.then((value) => callback(null, value), callback);
+```
+
+The second parameter of `then` can also be used to catch errors, but only errors
+from the existing promise, not the new one created by the callback.
+
+If the Deno equivalent is actually synchronous, there's a similar problem with
+try/catch statements:
+
+```ts
+try {
+  const value = process();
+  callback(null, value);
+} catch (err) {
+  callback(err);
+}
+```
+
+Since the callback is called within the `try` block, any errors from it will be
+caught and call the callback again.
+
+The correct way to do it is like this:
+
+```ts
+let err, value;
+try {
+  value = process();
+} catch (e) {
+  err = e;
+}
+if (err) {
+  callback(err); // Make sure arguments.length === 1
+} else {
+  callback(null, value);
+}
+```
+
+It's not as clean, but prevents the callback being called twice.