Make a proper Log class and methods #948
Conversation
Co-authored-by: will-morningstar <[email protected]>
- Put a wait in time.js - Modified review doc based on more findings
Co-authored-by: Russ741 <[email protected]>
- Ask questions - Experiment with missing logs on error. See https://replit.com/@plocket/Avoid-leaving-a-consoleConsole-file-blank-on-error#index.js - Describe new log-calling signature - Re-throw errors - Preserve log levels in debug console - Use `recursive` when creating artifacts folder Co-authored-by: will-morningstar <[email protected]> Co-authored-by: cthulahoops <[email protected]>
Also, update codes Co-authored-by: cthulahoops <[email protected]>
| `error`: | ||
| - Avoid using `error` explicitly. Only the debug logs should use the `error` level, and only to record critical errors that come in with `throw`. | ||
| `info`: | ||
| - Log expected errors as `info`. For example, the aborted promises when awaiting navigation. |
There was a problem hiding this comment.
Really interesting rule for what to use when. Trying to sit with my biases of what INFO is usually used for, and determine if it's worth renaming INFO to something else, like "suspicious" (SUS). Something that, by itself, can be ignored (and doesn't need to be investigated), but with a failure, might be an issue. Usually I'd use warn for that, but I do like warn being only for user focused logs
There was a problem hiding this comment.
I like this line of thought. No need to reply if you don't have bandwidth, I'll just brainstorm before.
Non-alarming words that can have an internal meaning:
- note
- detail
- cue
- queue
- data
- trace
- dig
- finding
- memo
- review
- intel
- mark
- comms
- signal
- task
- todo
Somewhat alarming words that can have an internal meaning:
- suspicious
- sus
- alert
- notice
- clue
- mark
- symptom
- advise
- caution
We could also require using "info" for things that don't need attention and leave "debug" as our internal signal to examine further. Or maybe, going in a different direction - just "alkiln"/"alk".
There was a problem hiding this comment.
Misread this note a bit. I think an alternative word (like "SUS") would be useful for internal warnings, but just want to be clear that an aborted promise is expected and is really just information - when one promise wins a race, the others will, of course, be (correctly) aborted. I added a design doc about the various meanings of "error" that might help clarify this a bit. I sort of want to combine that doc with this one, but I haven't seen how to make it clear.
| ## Non/anti-goals | ||
|
|
||
| - Spend a month on this | ||
| - Create a complex and heavy system to try to abstract absolutely everything |
There was a problem hiding this comment.
An additional non-goal that I think you might have; logs that are only understandable if you read the surrounding code. A lot of things can be simplified if you assume that, but it def raises the effort level needed by users quite a bit
Don't worry about looking at the code implementation for now. I'm doing a fair amount of refactoring. Thanks, @BryceStevenWilley! |
|
Ready, barring 1 main discussion point (and the docs). That one item - how to handle bugs internal to Thoughts:
While we mull over that, I'm going to keep plugging away at the design/decision docs. |
|
My current experiments to eliminate inner calls to |
#948 (comment) Co-authored-by: BryceStevenWilley <[email protected]>
Also: * Improve some logs * Implement logs in unit tests
- Add report_log.txt - Implement action output (untested) - Implement logs in unit tests Co-authored-by: will-morningstar <[email protected]>
Also tweak action log
- .console and .debug signatures diverge. Reconsider. - Always throw when calling `.throw` TODO: When we throw `null` we seem to not get a call stack. We gotta do something about that. Maybe always make an Error if `do_throw` is true Co-authored-by: will-morningstar <[email protected]>
Co-authored-by: will-morningstar <[email protected]>
tests, commit before error refactor Co-authored-by: will-morningstar <[email protected]>
reports, warn in debug on error, catch more. Note: writing to debug will write to unexpected
writing to unexpected file private, refactor to remove extra try/catches, match up log codes Tests: Try to neutralize console. Unable. Add new tests, add check string exclusion. Add notes
@ethanstrominger Co-authored-by: ethanstrominger <[email protected]>
- Ensure objects don't always log with a leading new line - Unify language around caught behavior/misbehavior to "Skipped" - Protect stdout - Tweak file descriptions (thanks Ethan!) Co-authored-by: ethanstrominger <[email protected]>
Co-authored-by: BryceStevenWilley <[email protected]>
#948 (comment) Co-authored-by: BryceStevenWilley <[email protected]>
* Add decision doc for getting and saving logs Thanks, @jder, for some extra experimentation with `console.Console` and write streams. * Describe the 4 environments in which ALKiln runs, tweak function descriptions in related files. * Update Log notes and descriptions * Tweak and clean up Log descriptions/comments, add TODOs * Focus log level choice doc on log levels instead of Log * Clarify types of errors and what to do with them. Dedup some info * Add more notes and content about logs Co-authored-by: jder <[email protected]>
#948 (comment) Co-authored-by: BryceStevenWilley <[email protected]>
* Continue filling documentation * Describe flow for artifact folder names in 4 environments * Clarify various docs
Apparently there was still a while to go. I still have decision and design/architecture docs to write, but this is ready for review. What I most want to discuss is how to handle bugs internal to
.debug().I wish this integrated the report more too, but this is already big enough.
In this PR, I have:
Reason for this PR
The logs have been a bit of a mess for a while. The verbose logs could only be logged live, not saved. Saving to the debug log was a bit add-hoc. There wasn't a good differentiation between console logs and debug logs. This addresses a few of those problems.
Links to any solved or related issues
Close #805, create verbose-type log
Close #924, remove extra url print
Close #659, abstract adding to debug_log
Close #925, allow a
Logto throw an errorAddress #461, setup and takedown reports
Any manual testing I have done to ensure my PR is working
Visually checked various log files.