René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"`fs.ReadFile` throws asynchronously with encoding on large enough input","articleBody":"Given 'utf8' encoding is specified, and a large enough input file, `fs.ReadFile` will throw an asynchronous error.\n#### Example program\n\n``` js\nvar fs = require('fs');\nfs.readFile('data.txt', 'utf8', function(err, data) {\n  console.log(err, data \u0026\u0026 data.length)\n})\n```\n#### Input: 268 megabytes of data\n\nSuccess.\n\n```\n\u003e dd if=/dev/zero of=data.txt  bs=1000000  count=268\n\u003e node -e \"fs = require('fs'); fs.readFile('data.txt', 'utf8', (err, data) =\u003e console.log(err, data \u0026\u0026 data.length))\"\nnull 268000000\n```\n#### Input: 269 megabytes of data\n\nNo Success.\nWorse, the error isn't forwarded to the callback – the process is throwing asynchronously.\n\n``` js\n\u003e dd if=/dev/zero of=data.txt  bs=1000000  count=269\n\u003e node -e \"fs = require('fs'); fs.readFile('data.txt', 'utf8', (err, data) =\u003e console.log(err, data \u0026\u0026 data.length))\"\nbuffer.js:359\n    throw new Error('toString failed');\n    ^\n\nError: toString failed\n    at Buffer.toString (buffer.js:359:11)\n    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:378:21)\n```\n\nThe error is not something specific to `fs.readFile` though, the same error is produced if we `toString` on the buffer directly, without the 'utf8' parameter, unsurprisingly. The problem is the uncatchable throw.\n\n``` js\n\u003e node -e \"fs = require('fs'); fs.readFile('data.txt', (err, data) =\u003e console.log(err, String(data).length))\"\n```\n\nWhile perhaps the throw in the `Buffer.prototype.toString` makes sense:\n\n``` js\nBuffer.prototype.toString = function() {\n  if (arguments.length === 0) {\n    var result = this.utf8Slice(0, this.length);\n  } else {\n    var result = slowToString.apply(this, arguments);\n  }\n  if (result === undefined)\n    throw new Error('toString failed');\n  return result;\n};\n```\n\nhttps://github.com/nodejs/node/blob/f8152df5e815c98be702fe58753258b58f02c4ad/lib/buffer.js#L352-L361\n\nIt does seem like poor behaviour to have uncatchable errors being thrown in core APIs where **the user has explicitly attached an errback**.\n\nWould a `try…catch` around the `buffer = buffer.toString(context.encoding);` in `fs.ReadFile` be appropriate?\n\n``` js\nfunction readFileAfterClose(err) {\n  var context = this.context;\n  var buffer = null;\n  var callback = context.callback;\n\n  if (context.err)\n    return callback(context.err);\n\n  if (context.size === 0)\n    buffer = Buffer.concat(context.buffers, context.pos);\n  else if (context.pos \u003c context.size)\n    buffer = context.buffer.slice(0, context.pos);\n  else\n    buffer = context.buffer;\n\n  // maybe ?\n  if (context.encoding) {\n    try {\n      buffer = buffer.toString(context.encoding); // currently this line is not wrapped in a try/catch\n    } catch (e) {\n      if (!err) err = e; // which error gets priority?\n    }\n  }\n\n  callback(err, buffer);\n}\n```\n\nhttps://github.com/nodejs/node/blob/f8152df5e815c98be702fe58753258b58f02c4ad/lib/fs.js#L377-L378\n\n`fs.readFile` is understandably hot code, I haven't done benchmarking on the impact of a `try…catch` here.\n\nGiven that there appears to be an upper limit to what can be stringified safely, perhaps the max length could be tested for before attempting the stringification, and a 'this is too big' Error would ideally pop out of the callback.\n\nSuggest at a minimum a more helpful error message \"our buffering is good but have you tried the streams?\"\n","author":{"url":"https://github.com/timoxley","@type":"Person","name":"timoxley"},"datePublished":"2015-09-09T11:19:53.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":13},"url":"https://github.com/2767/node/issues/2767"}