Skip to content
This repository has been archived by the owner on Aug 21, 2022. It is now read-only.

Disable console.error() #3

Closed
tkrotoff opened this issue Oct 29, 2015 · 5 comments
Closed

Disable console.error() #3

tkrotoff opened this issue Oct 29, 2015 · 5 comments

Comments

@tkrotoff
Copy link

A call to console.error() is present at line 22.

Would be nice to be able to disable it, especially when writing/running tests (can clutter the terminal output).

@blakeembrey
Copy link
Member

@tkrotoff If you're running into error.status >= 500, it's kind of an issue and you should look at fixing it to use better status code (>= 500 are server errors). I don't have any comment on whether there should have a flag to disable it though. In production, you definitely want to be notified of this.

@jonathanong
Copy link
Member

i don't mind a flag, but yeah, there's almost no reason these should be logged even in testing since you should be handling them.

@tkrotoff
Copy link
Author

As said, I'm not talking about production.

What is your workflow when working on an Express app? As of myself, I do write unit tests for failure cases like:

it('should not create a user without a name', done => {
  request(app).post('/Users/')
    .send({}) // Instead of {name: 'Edgar'}
    .expect(500) // Checks an error occurred
    .end((err, res) => {
      // blabla
    });
});

but then I end up with hundreds of lines like this one in my terminal (*):

error: insert into "Users" default values returning "id" - null value in column "name" violates not-null constraint
    at Connection.parseE (node_modules/pg/lib/connection.js:539:11)
    at Connection.parseMessage (node_modules/pg/lib/connection.js:366:17)
    at Socket.<anonymous> (node_modules/pg/lib/connection.js:105:22)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at readableAddChunk (_stream_readable.js:146:16)
    at Socket.Readable.push (_stream_readable.js:110:10)
    at TCP.onread (net.js:523:20)

lines that I don't care about in testing mode: jasmine/mocha already tells me if something is wrong.

(*) that don't occur when console.error() is commented

Edit: error 500 instead of 422

@jonathanong
Copy link
Member

well 1, that's a server error so it should be logging that error. your app should be handling the error.

2, it's not a 5xx error, so that that line in this module shouldn't be what's logging the error

@tkrotoff
Copy link
Author

@jonathanong

it's not a 5xx error

Pretend you did not see the 422 (I've edited the example) :)

that's a server error [...] your app should be handling the error

I see, my mistake, I should never ever have 5xx errors.

My code lazily pass all PostgreSQL errors to api-error-handler (.query().catch(error => next(error))) without a HTTP status code (so api-error-handler automatically sets it to 500).
Instead I should parse PostgreSQL errors and compute the proper HTTP status code.

To follow with the above example: if someone tries to create a user without a name, I should parse the PostgreSQL error and return 422 because of "Integrity Constraint Violation" (23502) instead of letting api-error-handler returning 500.

Hence my terminal cluttered by logs from api-error-handler, sorry for the bad report.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants