Make a proper Log class and methods #948

plocket · 2024-10-02T14:40:09Z

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:

Added tests for any new features or bug fixes (if relevant)
Added my changes to the CHANGELOG.md at the top, under the "Unreleased" section
Ensured issues that this PR closes will be automatically closed

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 Log to throw an error
Address #461, setup and takedown reports

Any manual testing I have done to ensure my PR is working

Visually checked various log files.

Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

- Put a wait in time.js - Modified review doc based on more findings

Co-authored-by: Russ741 <72242656+Russ741@users.noreply.github.com>

- 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 <mxwillmorningstar@gmail.com> Co-authored-by: cthulahoops <adam@cthulahoops.org>

Also, update codes Co-authored-by: cthulahoops <adam@cthulahoops.org>

BryceStevenWilley

Some initial thoughts. Didn't get a chance to look through everything, but focused on the design docs and how the new log API is used in Kiln itself.

BryceStevenWilley · 2024-10-15T21:16:34Z

docs/decisions/006_which_log_level_to_pick_2024_09_5.md

+`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.


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

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".

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.

lib/docassemble/server_install.js

docs/decisions/reports_and_logs_2024_08_11.md

lib/docassemble/docassemble_api_interface.js

BryceStevenWilley · 2024-10-16T03:53:40Z

docs/decisions/reports_and_logs_2024_08_11.md

+## Non/anti-goals
+
+- Spend a month on this
+- Create a complex and heavy system to try to abstract absolutely everything


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

plocket · 2024-10-16T14:23:11Z

Didn't get a chance to look through everything, but focused on the design docs and how the new log API is used in Kiln itself.

Don't worry about looking at the code implementation for now. I'm doing a fair amount of refactoring. Thanks, @BryceStevenWilley!

plocket · 2024-10-25T02:09:40Z

Ready, barring 1 main discussion point (and the docs). That one item - how to handle bugs internal to .debug(). They bring up the potential for infinite loops under very specific circumstances. The max_loops variable is a safety valve right now, but I hate it.

Thoughts:

I really don't like using max_loops or some other kind of cut-off. If we use it, we need to make it high enough that useful information doesn't get missed. That said, it just smells bad. Calling .debug() internally is just asking for trouble, even if we're calling it with what we believe to be safe arguments.
Option - collect the unexpected behavior messages as a list and, at the end, decide whether to save them to the debug log or to console log them. I'm not sure how to pass that data around, though.
Option - console log all unexpected behavior. I'm really reluctant to create that much noise for the authors when it's not-critical functionality.

While we mull over that, I'm going to keep plugging away at the design/decision docs.

plocket · 2024-10-28T15:05:21Z

My current experiments to eliminate inner calls to .debug() is to replace that call to use a function that abstracts saving straight to the debug file.

#948 (comment) Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

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 <mxwillmorningstar@gmail.com>

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 <mxwillmorningstar@gmail.com>

Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

tests, commit before error refactor Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

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

@ethanstrominger Co-authored-by: ethanstrominger <ethanstrominger2@gmail.com>

- 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 <ethanstrominger2@gmail.com>

Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

#948 (comment) Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

@jder

* 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 <me@jesserusak.com>

#948 (comment) Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

* Continue filling documentation * Describe flow for artifact folder names in 4 environments * Clarify various docs

plocket and others added 8 commits August 13, 2024 12:25

Brainstorm log goals and architecture

a285c80

Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

Implement draft of log.verbose() and

3f98f14

- Put a wait in time.js - Modified review doc based on more findings

Implement and lightly test basic log methods

f54bb43

Start replacing log with Log

6b669cd

Append to files, add error checking and fallbacks

4435bff

Co-authored-by: Russ741 <72242656+Russ741@users.noreply.github.com>

Update decisions, alias waiting, tweak logs

8e7e115

Implement new log interface for setup, run, takedown, da api,

0feaa11

Also, update codes Co-authored-by: cthulahoops <adam@cthulahoops.org>

plocket requested a review from BryceStevenWilley as a code owner October 2, 2024 14:40

BryceStevenWilley reviewed Oct 16, 2024

View reviewed changes

plocket force-pushed the 805_verbose branch from 114fd8f to 8f8d8df Compare October 16, 2024 17:47

plocket added a commit that referenced this pull request Oct 30, 2024

Make internal-specific icon and level, as per PR comment:

2123101

#948 (comment) Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

plocket force-pushed the 805_verbose branch from a31a3a3 to 7164f06 Compare November 1, 2024 13:01

plocket force-pushed the 805_verbose branch from 6a1a734 to 5839fc2 Compare November 8, 2024 13:11

plocket force-pushed the 805_verbose branch from 226c4a1 to 2fb1aaa Compare November 20, 2024 23:30

plocket and others added 12 commits November 21, 2024 10:36

Implement new log signature in all necessary files

c2f9e9a

Also: * Improve some logs * Implement logs in unit tests

Add logs decision doc, remove success rainbow from action

b6b5450

Change log methods signature, use fs.append, add stdout...

5fbe276

- Add report_log.txt - Implement action output (untested) - Implement logs in unit tests Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

Fix workflow typos, remove ALK0000, add TODOs, edit decision docs

9f4db4e

Add logging to actions

ed36ec4

Create folder and paths for log tests, file creation handles path

5a9c436

Also tweak action log

Refactor formatting, add tests, clean up

a3ee8cd

Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

Brainstorm about categories of errors, add notes, refactor, add

55391a0

tests, commit before error refactor Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

Remove explicitly writing to unexpected, note about writing to

9f3de78

reports, warn in debug on error, catch more. Note: writing to debug will write to unexpected

Only allow error on throw, warn of misuse, make

d97add8

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

Join stdout args with no spaces. Brainstorm class signature with

6d9ff35

@ethanstrominger Co-authored-by: ethanstrominger <ethanstrominger2@gmail.com>

plocket and others added 5 commits November 21, 2024 10:50

Tweak name, add function descriptions, comments, and examples

53904e4

Refactor deep methods for getting safe error

d8be814

Save both report and debug info to the unexpected results file

5df8518

Simplify use of context in wrapper functionality as per review

303a5dd

Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

plocket added a commit that referenced this pull request Nov 21, 2024

Make internal-specific icon and level, as per PR comment:

02e6e93

#948 (comment) Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

plocket force-pushed the 805_verbose branch from 2fb1aaa to 00e466c Compare November 21, 2024 15:52

plocket and others added 9 commits November 21, 2024 10:56

Abstract util.inspect()

92ed70d

Refactor to avoid internal circular calls of .debug()

fd62483

Implement some TODOs and clean up

9589791

Make internal-specific icon and level, as per PR comment:

59a416a

#948 (comment) Co-authored-by: BryceStevenWilley <Bryce.Steven.Willey@gmail.com>

Reformat and deduplicate reports, clarify var names

9935469

Continue work on documentation of logs architecture

36e06a0

* Continue filling documentation * Describe flow for artifact folder names in 4 environments * Clarify various docs

Rename doc about kinds of errors

056bb5b

Modify artifact folder name diagrams, clarify diagrams

e99c20d

plocket force-pushed the 805_verbose branch from 00e466c to 65bb728 Compare November 21, 2024 16:09

Add to CHANGELOG, correct/add diagrams. Mermaid needs "o" hack.

435b412

plocket force-pushed the 805_verbose branch from 65bb728 to 435b412 Compare November 22, 2024 13:39

Rename log.js to Log.js

976d66e

plocket force-pushed the 805_verbose branch from f19ba8a to 976d66e Compare November 22, 2024 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make a proper Log class and methods #948

Make a proper Log class and methods #948

plocket commented Oct 2, 2024 •

edited

Loading

BryceStevenWilley left a comment

BryceStevenWilley Oct 15, 2024

plocket Oct 16, 2024

plocket Oct 17, 2024

BryceStevenWilley Oct 16, 2024

plocket commented Oct 16, 2024

plocket commented Oct 25, 2024

plocket commented Oct 28, 2024

Make a proper Log class and methods #948

Are you sure you want to change the base?

Make a proper Log class and methods #948

Conversation

plocket commented Oct 2, 2024 • edited Loading

Reason for this PR

Links to any solved or related issues

Any manual testing I have done to ensure my PR is working

BryceStevenWilley left a comment

Choose a reason for hiding this comment

BryceStevenWilley Oct 15, 2024

Choose a reason for hiding this comment

plocket Oct 16, 2024

Choose a reason for hiding this comment

plocket Oct 17, 2024

Choose a reason for hiding this comment

BryceStevenWilley Oct 16, 2024

Choose a reason for hiding this comment

plocket commented Oct 16, 2024

plocket commented Oct 25, 2024

plocket commented Oct 28, 2024

plocket commented Oct 2, 2024 •

edited

Loading