console: do not emit error events #9744

addaleax · 2016-11-22T17:54:54Z

Checklist

make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

console

Description of change

Never fail for streams errors resulting from the console.* methods.

I know relatively little about the backstory here, so feel free to shoot this down quickly if it’s already been discussed and discarded.

/cc @Fishrock123

mcollina · 2016-11-22T18:21:40Z

lib/console.js

+      if (err) {
+        stream.once('error', () => {});
+      }
+    });


this 'error' function is added over and over again at every write.
I think the function should be added after stream.write synchronously, and then removed in the callback.
Beware of not allocating any new callbacks here, we will be adding strain to the gc for nothing (they can all be top level).

'error' function is added over and over again at every write.

Only if it fails? Like, maybe there’s a performance impact, but only in situations where the process current emits an uncaught exception.

I think the function should be added after stream.write synchronously, and then removed in the callback.

The error event is emitted after the callback, so I don’t think that works.

Beware of not allocating any new callbacks here, we will be adding strain to the gc for nothing (they can all be top level).

Done, thanks for pointing that out.

The error event is emitted after the callback, so I don’t think that works.

Then this PR creates a memory leak, and after a single console.log is used, any 'error' event will be swallowed.

@mcollina This is .once, not .on? If the callback sees an err argument, then that error will be emitted as an 'error' event, so the extra listener will be called and removed.

ooh, I did not notice, sorry. Add a comment there, so it's easy to understand what's going on.

mcollina

LGTM if CI is green.
Some comments needed to explain what's going on.

addaleax · 2016-11-22T20:13:57Z

Added a bit of text for clarification.

CI: https://ci.nodejs.org/job/node-test-commit/6136/

Fishrock123 · 2016-11-23T15:54:12Z

Previous related discussion at #9470

I'll take a look soon but this will also necessitate console.writeUnformatted/Raw so that people writing status bars etc can also avoid EPIPE and related issues.

Fishrock123 · 2016-11-23T15:54:56Z

That would make it admittedly less browser-like but browser conformity wasn't why I revived this in the first place.

addaleax · 2016-11-23T16:11:27Z

@Fishrock123 I’m not sure how I feel about that yet, but notice how this change could also be applied as an/in an userland module. I assume the set of packages (like progress bars) which want a “safe” behaviour is quite overseeable?

Fishrock123 · 2016-11-23T16:16:02Z

You can already do this in userland by just adding an error handler to stdio streams. The point is that it would be by default.

mcollina · 2016-11-23T17:28:12Z

@Fishrock123 I've always used direct process.stdout when dealing with progress bars. Is it customary to do it via console.log? Do you have some examples of this on npm?

Maybe those are two separate things, aren't they?

jasnell · 2016-12-05T22:55:38Z

lib/console.js

+        // an `error` event. Adding a `once` listener will keep that error
+        // from becoming an uncaught exception, but since the handler is
+        // removed after the event, non-console.* writes won’t be affected.
+        stream.once('error', noop);


hmm... can we guarantee this? For instance, if the callback ends up getting deferred by some buffering thing and some other error occurs that consumes the noop handler here....

node/lib/_stream_writable.js

Lines 338 to 347 in 0a265df

function onwriteError(stream, state, sync, er, cb) {

--state.pendingcb;

if (sync)

process.nextTick(cb, er);

else

cb(er);

stream._writableState.errorEmitted = true;

stream.emit('error', er);

}

… buffering or anything like that shouldn’t be the problem. But maybe this doesn’t work when the callback is called synchronously? I’d have to try it out but maybe @mcollina knows an answer off the top of his head?

(update: fixed link)

@addaleax, technically @jasnell has a point. You will need to wrap write in a once and removeListenerpair : in https://github.com/nodejs/node/blob/master/lib/_stream_writable.js#L340-L346, if sync is true the error is emitted before, then after.

ugh right, thanks for pointing out the “right” solution for this… updated and added tests for both scenarios

PTAL

mcollina · 2016-12-05T23:19:38Z

lib/console.js

+        // an `error` event. Adding a `once` listener will keep that error
+        // from becoming an uncaught exception, but since the handler is
+        // removed after the event, non-console.* writes won’t be affected.
+        stream.once('error', noop);


@addaleax, technically @jasnell has a point. You will need to wrap write in a once and removeListenerpair : in https://github.com/nodejs/node/blob/master/lib/_stream_writable.js#L340-L346, if sync is true the error is emitted before, then after.

addaleax · 2016-12-05T23:52:13Z

new CI: https://ci.nodejs.org/job/node-test-commit/6445/

mcollina

LGTM if CI is green

addaleax · 2016-12-13T18:31:00Z

@Fishrock123 Since you assigned this to yourself… what are the things you’d like to see/talk about here?

addaleax · 2017-01-04T16:27:07Z

ping @Fishrock123 ?

addaleax · 2017-01-19T07:51:08Z

@Fishrock123 It’s been a while… unless there are any objections, I’d like to land this in the next couple of days.

jasnell · 2017-01-19T16:32:44Z

lib/console.js

+    // Add and later remove a noop error handler to catch synchronous errors.
+    stream.once('error', noop);
+
+    stream.write(string, (err) => {


Rather than allocating a closure here, can this be made a top level function?

It can’t be top-level because it still needs access to the stream but I’ve attached these to the console object itself, does that work for you? It definitely avoids the closure problem.

jasnell · 2017-01-19T16:35:29Z

lib/console.js

+  } catch (e) {
+    // Sorry, there’s no proper way to pass along the error here.
+  } finally {
+    stream.removeListener('error', noop);


Hmm... going back to the earlier point, is the write operation guaranteed to be complete by the time we hit this?

No but that’s not a problem. Asynchronous errors are handled from inside the callback.

jasnell · 2017-01-19T16:38:17Z

test/parallel/test-console-sync-write-error.js

@@ -0,0 +1,17 @@
+'use strict';


It would be helpful to have additional tests that verify this works with a variety of stdout/stderr configurations (e.g. stdio as a file, stdio as a pipe, etc)

I’ve updated this with tests for something that errors asynchronously… the inner workings of console aren’t really tied to I/O at all, so I’m not sure it makes sense to add explicit tests for these

The main thing that I'm hoping for is that this would fix the issue reported in #831

addaleax · 2017-01-19T17:37:04Z

CI: https://ci.nodejs.org/job/node-test-commit/7361/

addaleax · 2017-01-25T21:03:05Z

@jasnell please take another look when you have the time :)

jasnell · 2017-01-27T21:19:24Z

Reviewing now...

jasnell

A minor nit but generally LGTM

jasnell · 2017-01-27T21:42:31Z

test/parallel/test-console-async-write-error.js

+const assert = require('assert');
+
+const out = new Writable({
+  write(chunk, enc, callback) {


Just to be on the safe side, there should likely be a check that verifies this version of write is being called. I'm not certain if common.mustCall() would work here.

@jasnell Done, good point. (Sorry for being late, must have overlooked your replies.)

Fixes: nodejs#831 Fixes: nodejs#947 Ref: nodejs#9470

thefourtheye · 2017-02-12T17:40:32Z

test/parallel/test-console-sync-write-error.js

+{
+  const out = new Writable({
+    write(chunk, enc, callback) {
+      callback(new Error('foobar'));


It would be better if we could somehow mustCall all these.

@thefourtheye done!

addaleax · 2017-02-15T16:13:32Z

CI: https://ci.nodejs.org/job/node-test-commit/7925/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/576/
CITGM for master: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/578/

jasnell · 2017-02-15T17:40:52Z

Still LGTM and +1 to semver-major

addaleax · 2017-02-15T20:30:55Z

Landed in f18e08d

Fixes: #831 Fixes: #947 Ref: #9470 PR-URL: #9744 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Fishrock123 · 2017-02-15T21:19:18Z

I still hold that this requires us to implement a console function with equivalent functionality to process.stdout.write() but with this safety.

addaleax · 2017-02-15T21:21:21Z

@Fishrock123 Sorry, what exactly are you suggesting?

Fishrock123 · 2017-02-15T21:33:02Z

I will make a PR shortly. (hopefully)

Apply the `console: do not emit error events` changes properly to `console.dir()`. This was overlooked in f18e08d (nodejs#9744). Ref: nodejs@f18e08d#commitcomment-20934407

Apply the `console: do not emit error events` changes properly to `console.dir()`. This was overlooked in f18e08d (#9744). Ref: f18e08d#commitcomment-20934407 PR-URL: #11443 Reviewed-By: James M Snell <jasnell@gmail.com>

litmit · 2017-02-28T07:20:21Z

This PR possible problematic.

After commit f18e08d a lot of calls console.log() cause memory leaks.

See #11568 for details.

* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](nodejs@4a7233c178)] [nodejs#12892](nodejs#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](nodejs@d2d32ea5a2)] [nodejs#11968](nodejs#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](nodejs@7eb1b4658e)] [nodejs#12141](nodejs#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](nodejs@beca3244e2)] [nodejs#10236](nodejs#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](nodejs@97a77288ce)] [nodejs#12348](nodejs#12348), [[`d75fdd96aa`](nodejs@d75fdd96aa)] [nodejs#10423](nodejs#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](nodejs@627ecee9ed)] [nodejs#10653](nodejs#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](nodejs@f18e08d820)] [nodejs#9744](nodejs#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](nodejs@3c3b36af0f)] [nodejs#12936](nodejs#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](nodejs@60d1aac8d2)] [nodejs#12784](nodejs#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](nodejs@84dabe8373)] [nodejs#12489](nodejs#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](nodejs@7a55e34ef4)] [nodejs#10467](nodejs#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](nodejs@3c2a9361ff)] [nodejs#9683](nodejs#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](nodejs@90403dd1d0)] [nodejs#11567](nodejs#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](nodejs@d3480776c7)] [nodejs#11259](nodejs#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](nodejs@fb71ba4921)] [nodejs#11355](nodejs#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](nodejs@3e6f1032a4)] [nodejs#10805](nodejs#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](nodejs@5de3cf099c)] [nodejs#10116](nodejs#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](nodejs@84a23391f6)] [nodejs#12113](nodejs#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](nodejs@56e881d0b0)] [nodejs#11975](nodejs#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](nodejs@03e89b3ff2)] [nodejs#10116](nodejs#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](nodejs@dd20e68b0f)] [nodejs#12725](nodejs#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](nodejs@3f27f02da0)] [nodejs#11599](nodejs#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (nodejs@ec7cbaf266)] [nodejs#12995](nodejs#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](nodejs@a16b570f8c)] [nodejs#11968](nodejs#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](nodejs@010f864426)] [nodejs#12949](nodejs#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](nodejs@a5f91ab230)] [nodejs#11689](nodejs#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](nodejs@8a7db9d4b5)] [nodejs#12087](nodejs#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](nodejs@b6e1d22fa6)] [nodejs#12925](nodejs#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](nodejs@07c7f198db)] [nodejs#12828](nodejs#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](nodejs@348cc80a3c)] [nodejs#5923](nodejs#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](nodejs@a2ae08999b)] [nodejs#11349](nodejs#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](nodejs@d523eb9c40)] [nodejs#11447](nodejs#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](nodejs@d080ead0f9)] [nodejs#12710](nodejs#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](nodejs@5bfd13b81e)] [nodejs#9726](nodejs#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](nodejs@455e6f1dd8)] [nodejs#11708](nodejs#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](nodejs@aab0d202f8)] [nodejs#11624](nodejs#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](nodejs@99da8e8e02)] [nodejs#12442](nodejs#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](nodejs@91383e47fd)] [nodejs#12001](nodejs#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](nodejs@b514bd231e)] [nodejs#11391](nodejs#11391).

* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](4a7233c178)] [#12892](#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](d2d32ea5a2)] [#11968](#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](7eb1b4658e)] [#12141](#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](beca3244e2)] [#10236](#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](97a77288ce)] [#12348](#12348), [[`d75fdd96aa`](d75fdd96aa)] [#10423](#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](627ecee9ed)] [#10653](#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](f18e08d820)] [#9744](#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](3c3b36af0f)] [#12936](#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](60d1aac8d2)] [#12784](#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](84dabe8373)] [#12489](#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](7a55e34ef4)] [#10467](#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](3c2a9361ff)] [#9683](#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](90403dd1d0)] [#11567](#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](d3480776c7)] [#11259](#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](fb71ba4921)] [#11355](#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](3e6f1032a4)] [#10805](#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](5de3cf099c)] [#10116](#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](84a23391f6)] [#12113](#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](56e881d0b0)] [#11975](#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](03e89b3ff2)] [#10116](#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](dd20e68b0f)] [#12725](#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](3f27f02da0)] [#11599](#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (ec7cbaf266)] [#12995](#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](a16b570f8c)] [#11968](#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](010f864426)] [#12949](#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](a5f91ab230)] [#11689](#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](8a7db9d4b5)] [#12087](#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](b6e1d22fa6)] [#12925](#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](07c7f198db)] [#12828](#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](348cc80a3c)] [#5923](#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](a2ae08999b)] [#11349](#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](d523eb9c40)] [#11447](#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](d080ead0f9)] [#12710](#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](5bfd13b81e)] [#9726](#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](455e6f1dd8)] [#11708](#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](aab0d202f8)] [#11624](#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](99da8e8e02)] [#12442](#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](91383e47fd)] [#12001](#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](b514bd231e)] [#11391](#11391).

nodejs-github-bot added the console Issues and PRs related to the console subsystem. label Nov 22, 2016

addaleax mentioned this pull request Nov 22, 2016

process: swallow stdout/stderr error events #9470

Closed

4 tasks

addaleax force-pushed the console-no-errors2 branch from 299f647 to e0c476d Compare November 22, 2016 17:57

mcollina requested changes Nov 22, 2016

View reviewed changes

addaleax force-pushed the console-no-errors2 branch from e0c476d to b04283b Compare November 22, 2016 18:59

mcollina approved these changes Nov 22, 2016

View reviewed changes

Fishrock123 mentioned this pull request Dec 5, 2016

Proposal: add a console.write method #10119

Closed

Fishrock123 self-assigned this Dec 5, 2016

jasnell reviewed Dec 5, 2016

View reviewed changes

mcollina requested changes Dec 5, 2016

View reviewed changes

addaleax force-pushed the console-no-errors2 branch from e883c56 to 1e4f914 Compare December 5, 2016 23:50

mcollina approved these changes Dec 6, 2016

View reviewed changes

jasnell requested changes Jan 19, 2017

View reviewed changes

addaleax force-pushed the console-no-errors2 branch from 1e4f914 to 9bc88af Compare January 19, 2017 17:17

jasnell approved these changes Jan 27, 2017

View reviewed changes

addaleax unassigned Fishrock123 Feb 11, 2017

console: do not emit error events

43e9611

Fixes: nodejs#831 Fixes: nodejs#947 Ref: nodejs#9470

thefourtheye reviewed Feb 12, 2017

View reviewed changes

addaleax added 2 commits February 12, 2017 18:47

[squash] add more common.mustCalls

8851777

[squash] use default value for ignoreErrors parameter

8b46d5e

thefourtheye approved these changes Feb 12, 2017

View reviewed changes

addaleax closed this Feb 15, 2017

addaleax deleted the console-no-errors2 branch February 15, 2017 21:20

addaleax mentioned this pull request Feb 17, 2017

console: fixup console.dir() error handling #11443

Closed

3 tasks

litmit mentioned this pull request Feb 28, 2017

Any way to to turn console into fully synchronous? #11568

Closed

jasnell mentioned this pull request Apr 4, 2017

8.0.0 Release Proposal #12220

Closed

addaleax mentioned this pull request May 14, 2017

EPIPE errors thrown when piping to a closed stream #947

Closed

ronag mentioned this pull request Aug 24, 2019

stream: use callback to properly propagate error #29179

Closed

4 tasks

anko mentioned this pull request May 6, 2020

console.log no longer EPIPEs in Node@14 (but process.stdout.write does) mhart/epipebomb#10

Open

	function onwriteError(stream, state, sync, er, cb) {
	--state.pendingcb;
	if (sync)
	process.nextTick(cb, er);
	else
	cb(er);

	stream._writableState.errorEmitted = true;
	stream.emit('error', er);
	}

console: do not emit error events #9744

console: do not emit error events #9744

Conversation

addaleax commented Nov 22, 2016

Checklist

Affected core subsystem(s)

Description of change

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

addaleax commented Nov 22, 2016

Fishrock123 commented Nov 23, 2016

Fishrock123 commented Nov 23, 2016

addaleax commented Nov 23, 2016

Fishrock123 commented Nov 23, 2016

mcollina commented Nov 23, 2016

Choose a reason for hiding this comment

addaleax Dec 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax commented Dec 5, 2016

mcollina left a comment

Choose a reason for hiding this comment

addaleax commented Dec 13, 2016

addaleax commented Jan 4, 2017

addaleax commented Jan 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax commented Jan 19, 2017

addaleax commented Jan 25, 2017

jasnell commented Jan 27, 2017

jasnell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax commented Feb 15, 2017 • edited Loading

jasnell commented Feb 15, 2017

addaleax commented Feb 15, 2017

Fishrock123 commented Feb 15, 2017

addaleax commented Feb 15, 2017

Fishrock123 commented Feb 15, 2017

litmit commented Feb 28, 2017 • edited Loading

addaleax Dec 5, 2016 •

edited

Loading

addaleax commented Feb 15, 2017 •

edited

Loading

litmit commented Feb 28, 2017 •

edited

Loading