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

console.log in controllers #1541

Closed
sujeethk opened this issue Oct 3, 2016 · 3 comments
Closed

console.log in controllers #1541

sujeethk opened this issue Oct 3, 2016 · 3 comments

Comments

@sujeethk
Copy link
Contributor

sujeethk commented Oct 3, 2016

Are these console.log in code necessary?

users.password.server.controller.js#L65
users.password.server.controller.js#L71
articles.client.service.js - harmless yet unnecessary

Especially the one on the password server controller. It is logging the user's password reset link. I would assume it is so that one could test the reset flow even without MSP. But on a production env wouldn't it be a security issue?

@mleanos
Copy link
Member

mleanos commented Oct 3, 2016

I think the one in the Articles client-service can stay. Of course I would think that since I'm the one that added that code :) Since we're not handling that error internally to the service, we can output it to the console; it should make debugging/support easier. Ideally, we should handle it in some way but that would most likely vary on the project.

The other console logs you listed can be removed.

@simison
Copy link
Member

simison commented Oct 3, 2016

I think the one in the Articles client-service can stay. Of course I would think that since I'm the one that added that code :)

Tho it really should be using Angular's $log instead of console:

  1. $log can be set silent from Angular configuration (i.e. for production).
  2. console doesn't work on some weird or old browsers

First two should definetely be removed, just adds garbage to logs.

@mleanos
Copy link
Member

mleanos commented Oct 3, 2016

@simison Agreed. I know you mentioned the use of $log before too. We should make this a standard.

mleanos pushed a commit that referenced this issue Oct 9, 2016
logs with $log in client idea proposed by @simison
also removed unnecessary consoles in users.password.server.controller.js

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

No branches or pull requests

3 participants