Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide which of the current console messages are to be production/development/debug environment #430

Open
Martii opened this issue Nov 18, 2014 · 9 comments
Labels
team biz This is similar to a meta discussion.

Comments

@Martii
Copy link
Member

Martii commented Nov 18, 2014

Currently we have a bunch of console.log, console.warn and console.err lines in our code... decisions need to be made on which is to be applied to the isPro, isDev and isDbg flag tests.

Also needed are some more console messages to actually point out the flow of our MVC. Please discuss this here.

@Martii Martii added the team biz This is similar to a meta discussion. label Nov 18, 2014
@Zren
Copy link
Contributor

Zren commented Nov 18, 2014

Just leave them all. This is pretty much a non-issue.

@Martii
Copy link
Member Author

Martii commented Nov 20, 2014

Just leave them all.

All of the above is an option... however since you aren't reading the deployment logs this may not be an option.

This is pretty much a non-issue.

Perhaps from a narrow minded perspective but you might want to consider someone else besides just you.

@jerone
Copy link
Contributor

jerone commented Nov 20, 2014

@Martii commented on 20 nov. 2014 06:43 CET:

Perhaps from a narrow minded perspective but you might want to consider someone else besides just you.

There's no one else to discuss this with, apparently you are the only one with access to 'these so called logs'.

@Martii
Copy link
Member Author

Martii commented Nov 20, 2014

No sizzle is the one with that access from what I can tell and I don't know exactly where he dug one up he sent me... there's also hosting support that definitely has the logs... so yes it is more than just one person.

@Martii
Copy link
Member Author

Martii commented Nov 20, 2014

Everyone is also forgetting dev/dbg is one user maybe two... whereas production could be hundreds or more... so selectively choosing what shows and what doesn't when and where is a good thing.

@Zren
Copy link
Contributor

Zren commented Nov 20, 2014

Any logging in master is production logging. If you have dev/debug logging, it should be removed or commented out before being merged.

I agree that there should be more logging during user initiated events, but it's not terribly necessary unless it's required to debug something in production and/or to count how many times an event happens. Basically, if you notice something should be logged when you're refactoring, insert some logging in it when you make your PR.

Likewise, if you notice something is spamming, make a PR to remove/comment it out. There isn't really a need to discuss this problem in general since every use case is different.

@Martii
Copy link
Member Author

Martii commented Dec 2, 2014

@Zren
It is painfully obvious that you don't have an understanding of what is going on as of late and have been fighting the project for some time now. It's great to share your opinion however your limited ability to see the larger picture is proving to be a hindrance.

Production has debugging whether you like it or not... your misconception of this process really needs to lay low for a while... and to quote someone else you are completely and absolutely incorrect on your interpretation of this. Even if you bring up all the bad examples from node projects to "prove" your point there is absolutely no beneficial reasoning behind limiting production with an arcane and immature decision by stripping debugging. I have already proven that production has issues that can't be resolved in development nor debug mode... so have you but you choose to ignore the facts.

Anyhow... I'll be working on this in the next few days... with any luck.

@Zren
Copy link
Contributor

Zren commented Dec 2, 2014

apparently you are the only one with access to 'these so called logs'.

Which makes it difficult to know for those only used to logging output in dev environments.

no beneficial reasoning behind limiting production with an arcane and immature decision by stripping debugging

I was talking about removing "the code has reached enteredMethodA()" to debug broken logic errors when code is being written, after the code is considered bug free and before a PR is made. I did not mean we should remove logging required to find bugs in production.

  • Anything logged in production does not need an isPro check as you should be logging it in development as well.
  • Anything logged in development does not need an isDev check as if it's not making it into master aka production then you can just use basic console.log() statements which you can delete (or comment out if they're rather useful) before commiting them.

@Martii
Copy link
Member Author

Martii commented Dec 2, 2014

Which makes it difficult to know for those only used to logging output in dev environments.

I would like you to go read my "fluff", as you put it a while back, for jitsu. This has been referenced on the main page for a bit with the README.md... in particular read this section.

I've also shared with you #425 which is part of the host... nodejitsu isn't your traditional virtual nix host... it has its own front end and it's own back end.

I've also shared that the pro has an environment that is displayed like a JSON file... e.g. we can set the configuration... so please don't act like you don't know what's going on... you have been shared with.

You are one of the smartest/quickest coders I've seen but you are also the least focused that I've seen in a while... don't let me down by missing the points... ask a question if you need one from me and I'll do the best that I can to answer it.

Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Nov 11, 2015
* Also trying out *chalk* dep to see if the VPS pro logs can handle this without too much visual interference... nicer in development and debug modes... if not will remove.

Applies to Code Migrations, OpenUserJS#430,  and jaredhanson/passport#400 (comment)

Still getting `username is taken` with *passport*@0.3.2 and *passport-github*@1.0.0
but **not** *passport*@0.2.2 with *passport-github*@0.1.5
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Dec 16, 2015
* Set some DB options to their assumed defaults from http://mongodb.github.io/node-mongodb-native/2.0/api/Server.html ... relates to https://github.com/christkv/mongodb-core/issues/66#issuecomment-165052045
* Set some replset defaults to their assumed defaults from http://mongodb.github.io/node-mongodb-native/2.0/api/ReplSet.html ... relates to Automattic/mongoose#3588 (comment)
* Move/change a stderr message to debug mode of *node*... clear up the logs a bit since script source may not be found just yet. This only happens on pro so far and probably deals with streams not ready yet. Applies to OpenUserJS#430
* Move S3 source not found stderr to debug mode of *node*... known issue of OpenUserJS#486. Applies to OpenUserJS#430

**NOTES**
* This will be a test whereas unless it's a huge bug I can't take pro down for at least 3 days, give or take to see if this helps or hinders from OpenUserJS#845 backout.
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Feb 21, 2016
* Remove legacy namespace in all routes ... part of OpenUserJS#262 and OpenUserJS#130... officially EOL as per sizzle and confirmed by me... no redirects.
* Remove the patches I put in for this portion
* Create function for encoding similar to GH's... see helper `encoding`
* Using encodeURIComponent from OpenUserJS#795
* Fix bug of Fork History showing "slugged" instead of native/raw
* Fix category reference bug... redirect was set to the Object itself and not one of its properties
* Fix some more missing object identifiers in modelParser.js ... not all are validated yet and some may still be missing for future proofing
* Remove and Add some more watchpoints
* Flip successful "rating updated" for Groups to debug mode and related in `./libs/modelParser.js` ... these are generating too much log traffic OpenUserJS#430

**NOTES**
* [ ] Followup with DB migration to add `fork.utf` to existing forks otherwise they may not show up next to the bullet
* Leaving GH import alone as it still works so far ... however a watchpoint added... GH web UI always `encodeURIComponent` just about everything... will need to reaffirm existing `encodeURI` later
* The term uri is inclusive to urls as previously described in OpenUserJS#819 ... however in this context url means it could be partially encoded whereas uri means it absolutely is encoded... except for the slashes... normally those are encoded but we don't want them encoded... again following GH example. A url containing `ä` may be just that however a uri has `%E4`. You will notice that I renamed some of our identifiers from `url` to `uri`... this is on purpose to indicate that is what we are expecting.
* *express* `redirect()` uses [`Location`](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30) and [`Content-Location`](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.14) headers. This requires all urls to be at least URI encoded as per specs... e.g. all the browsers are adding in an `escape` for the section symbol *which may be an issue with them*. So hence why the naming of `url` vs `uri`.
* The base issue of readability of the code is still there... but at least this punches out some of the bugs mentioned.
* Compression/optimization hasn't occurred yet
* No DB migration is necessary now... this includes fixing `Re:` linkage
* The smarter encoder isn't finished yet as I may be rearranging this a bit but...

**This should knock out the BLOCKING label I put on**... at least from my perspective.

Applies to OpenUserJS#819 and OpenUserJS#262... treads on OpenUserJS#262 (comment) among many others.
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Feb 24, 2016
* Post first 100 chars of body for relase *express-minify* on error... id what's occasionally happening. Hopefully this is enough.
* Remove logging from OpenUserJS#901... got some data and posted Unit Test data at https://github.com/OpenUserJs/OpenUserJS.org/pull/901/files#r53597717
* Expand single removal message to be more specific

Applies to OpenUserJS#430
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Apr 19, 2017
* Some STYLEGUIDE.md conformance in this function

Applies to OpenUserJS#430
Martii added a commit that referenced this issue Apr 19, 2017
* Some STYLEGUIDE.md conformance in this function

Applies to #430 

Auto-merge
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Apr 24, 2017
* Since OpenUserJS#1086 haven't seen any of these yet for OpenUserJS#486
* Use an environment variable to toggle monitoring this for now


Applies to OpenUserJS#430
Martii added a commit that referenced this issue Apr 24, 2017
* Since #1086 haven't seen any of these yet for #486
* Use an environment variable to toggle monitoring this for now


Applies to #430

Auto-merge
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue May 1, 2017
* On rare occasions S3 connection goes down so improve error logging... UI will currently show 404 on script edit submit... ideally this should be 502 but will work as former. UI new scripts will bail out and reload empty editor.
* MongoDB, once inside `.save` it will **not** abort unless it's a `.pre` option with the schema which we don't currently utilize due to code structure so do it last.
* Improve logging for MongoDB errors... if MongoDB connection goes down script source will be saved but UI will be out of sync instead of completely absent on a request... these are even rarer than S3.
* Denoted `fallthrough` comment on an error trap if User elevation didn't happen but log for Admin+ intervention.

NOTES:
`unmarkModified` is pretty much useless in this case scenario but was tested

Applies to OpenUserJS#430 / OpenUserJS#72 Related to OpenUserJS#486
Martii added a commit that referenced this issue May 1, 2017
* On rare occasions S3 connection goes down so improve error logging... UI will currently show 404 on script edit submit... ideally this should be 502 but will work as former. UI new scripts will bail out and reload empty editor.
* MongoDB, once inside `.save` it will **not** abort unless it's a `.pre` option with the schema which we don't currently utilize due to code structure so do it last.
* Improve logging for MongoDB errors... if MongoDB connection goes down script source will be saved but UI will be out of sync instead of completely absent on a request... these are even rarer than S3.
* Denoted `fallthrough` comment on an error trap if User elevation didn't happen but log for Admin+ intervention.

NOTES:
`unmarkModified` is pretty much useless in this case scenario but was tested

Applies to #430 / #72 Related to #486

Auto-merge
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue May 3, 2017
* Rework/revisit *toobusy-js* a little to improve middleware performance... does slow start up time a bit
* Some immediate returns added instead of just fallthrough

**NOTES**
* Discovered VPS memory size change **decrease** :\

Applies to OpenUserJS#944 and OpenUserJS#430
Martii added a commit that referenced this issue Jul 6, 2018
* Adjust values to shorter, preferred, intervals
* Additional sanity check for idle session from browser blocked and query accept redirect with JavaScript disabled.
* Restore #1448 *(not found in stderr and new users are okay)*... ends mitigation from #1449 ... most likely a critical DB failure with mongolabs that should be trapped and handled
* Move destruction to stderr with debug mode only... Applies to #430

NOTE:
* Unit specific tests... if changed must keep in `m` and `h`. `nominal` replacement and `maximum` addition are in `h`... the rest are `m`

Post #1471 ... related to #604

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Sep 24, 2018
* Move `console` output to dev only for experimental minification

Applies to OpenUserJS#432 OpenUserJS#249 OpenUserJS#430
Martii added a commit that referenced this issue Sep 24, 2018
* Move `console` output to dev only for experimental minification

Applies to #432 #249 #430

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Oct 14, 2018
Martii added a commit that referenced this issue Oct 14, 2018
* Add in some dev console messages to assist

Post #1528 #1438 ; Applies to #430 ; See also highlightjs/highlight.js#1868

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Dec 21, 2018
* These are mostly comments in this PR
* Normalized a few more identifiers to their standards for easier grep'ing. I'm sure there's more.
* Some of these may be silent as intended... they should be denoted as such... like `fallthrough` for `switch` should be normally
* Some of these assume that AWS S3 and MongoLabs are perfectly connected and no issues... bad idea. This is what I was describing. We need [OpenUserJS#37](Graceful failure) whether it's a comment, a console message, a page, or some combination of these.
* This doesn't include MongoDB calls that just use `save()` for example... which is not recommended at all. Error traps are good when connecting to third parties whether they are local or remote. I'm not perfect either but I can get easily burned out on fixing these which is why I work in stages instead of all at once. Don't get paid for that! ;) :) Always fill out the arguments if known especially with `aErr`. That's a reason why it's done first in this coding style.
* This was a very brief skim over the code... hopefully it wasn't overzealous or underzealous in code points... but they are comments to be addressed eventually. Living on four hours of sleep in 3 days isn't good so I'm taking a break not to mention a few days away from holiday days.

Applies to OpenUserJS#430 and indirectly to OpenUserJS#1548

Signed-off-by: Martii <martii@users.noreply.github.com>
Martii added a commit that referenced this issue Dec 21, 2018
* These are mostly comments in this PR
* Normalized a few more identifiers to their standards for easier grep'ing. I'm sure there's more.
* Some of these may be silent as intended... they should be denoted as such... like `fallthrough` for `switch` should be normally
* Some of these assume that AWS S3 and MongoLabs are perfectly connected and no issues... bad idea. This is what I was describing. We need [#37](Graceful failure) whether it's a comment, a console message, a page, or some combination of these.
* This doesn't include MongoDB calls that just use `save()` for example... which is not recommended at all. Error traps are good when connecting to third parties whether they are local or remote. I'm not perfect either but I can get easily burned out on fixing these which is why I work in stages instead of all at once. Don't get paid for that! ;) :) Always fill out the arguments if known especially with `aErr`. That's a reason why it's done first in this coding style.
* This was a very brief skim over the code... hopefully it wasn't overzealous or underzealous in code points... but they are comments to be addressed eventually. Living on four hours of sleep in 3 days isn't good so I'm taking a break not to mention a few days away from holiday days.

Applies to #430 and indirectly to #1548

Signed-off-by: Martii <martii@users.noreply.github.com>

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Feb 10, 2019
* Script up to down or down to up improperly counts script votes and messes up good/bad bar. Shouldn't affect Rating in Script or Group but will recheck a few.
* At the same time move to MVC *(Currently isolated to Script e.g. parallel change)*... Fixes TODO in routes.js
* Some styleguide.md conformance
* Simplify some views. e.g. turn some things off when they aren't able to be used. Save b/w and possibly improve understanding for most
* Still would prefer returning `err` *(aErr)* with libs but that's probably a separate PR since there are still inconsistent return callback parms
* Some filling in for OpenUserJS#1548
* Some additions for OpenUserJS#430. e.g. we want to know if a DB action fails currently

NOTES:
* Structure mirrored and altered from `flagsLib`
Applies to OpenUserJS#262 and Active Maintainer override (Sorry sizzle but needed to be done so it doesn't mess up other areas down the line)
* Will run through scripts and mitigate any script `votes` counts.
Martii added a commit that referenced this issue Feb 10, 2019
* Script up to down or down to up improperly counts script votes and messes up good/bad bar. Shouldn't affect Rating in Script or Group but will recheck a few.
* At the same time move to MVC *(Currently isolated to Script e.g. parallel change)*... Fixes TODO in routes.js
* Some styleguide.md conformance
* Simplify some views. e.g. turn some things off when they aren't able to be used. Save b/w and possibly improve understanding for most
* Still would prefer returning `err` *(aErr)* with libs but that's probably a separate PR since there are still inconsistent return callback parms
* Some filling in for #1548
* Some additions for #430. e.g. we want to know if a DB action fails currently

NOTES:
* Structure mirrored and altered from `flagsLib`
Applies to #262 and Active Maintainer override (Sorry sizzle but needed to be done so it doesn't mess up other areas down the line)
* Will run through scripts and mitigate any script `votes` counts.

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Aug 22, 2020
* This is used for Authors to track the internal processing status of each script.
* Only shows up per Author i.e. nobody else and you *must* be logged in to view
* Search might be incorrect atm... still tinkering with that part.
* Hide some pre-existing GH items if no auth strategy present.
* Eventually more is needed but this is a start.

Applies to OpenUserJS#1730 and a few related others in the past; Will eventually affect OpenUserJS#430 by moving debugging to Author page instead of OUJS intervention.
Martii added a commit that referenced this issue Aug 22, 2020
* This is used for Authors to track the internal processing status of each script.
* Only shows up per Author i.e. nobody else and you *must* be logged in to view
* Search might be incorrect atm... still tinkering with that part.
* Hide some pre-existing GH items if no auth strategy present.
* Eventually more is needed but this is a start.

Applies to #1730 and a few related others in the past; Will eventually affect #430 by moving debugging to Author page instead of OUJS intervention.

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Sep 22, 2020
* Better error handling for Write Script Online.
* Better error handling for Upload Script. Fixes potential server trips and/or hangings.
* Repo URL change detected on GH for *formidable*... updated README.md linkage.
* Some symmetry in these areas.

Post OpenUserJS#1730 OpenUserJS#430 OpenUserJS#37
Martii added a commit that referenced this issue Sep 22, 2020
* Better error handling for Write Script Online.
* Better error handling for Upload Script. Fixes potential server trips and/or hangings.
* Repo URL change detected on GH for *formidable*... updated README.md linkage.
* Some symmetry in these areas.

Post #1730 #430 #37

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Sep 25, 2020
* Catch GH import routine
* Add `#bypass` to statusCodePage

Post OpenUserJS#1731 , applies to OpenUserJS#430 OpenUserJS#37 and closes OpenUserJS#1730
Martii added a commit that referenced this issue Sep 25, 2020
* Catch GH import routine
* Add `#bypass` to statusCodePage

Post #1731 , applies to #430 #37 and closes #1730

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Dec 8, 2021
Martii added a commit that referenced this issue Dec 8, 2021
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Jan 15, 2022
Martii added a commit that referenced this issue Jan 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team biz This is similar to a meta discussion.
Development

No branches or pull requests

3 participants