-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(gatsby-cli): log what activities prevent from transitioning to idle when stuck #26618
feat(gatsby-cli): log what activities prevent from transitioning to idle when stuck #26618
Conversation
…vity object in redux state
… (add actual log action there)
Gatsby Cloud Build Reportusing-styled-components 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 12s PerformanceLighthouse report
|
Gatsby Cloud Build Reportclient-only-paths 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 12s PerformanceLighthouse report
|
Gatsby Cloud Build Reportusing-reach-skip-nav 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 12s PerformanceLighthouse report
|
Gatsby Cloud Build Reportgatsby 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 8m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have it as Redux middleware? It sounds like a good fit for something like this?
It kind of is in the "middleware" - all structured logs related actions go through it (but also worth mentioning that this is resetting timer only on structured logging actions). It's just it's not using builtin redux middleware API because logic there for the most part only relates to logging (deciding whether to forward action to ipc) and it supports changing the target store instance (it always starts with store that is local to gatsby-cli, but can swap to gatsby store). By using redux middleware did you mean to use proper middleware API or that it should reset timer on any action (doesn't have to be structured logging one)? Or both? |
Oh, right, I forgot about swapping stores thing. Discard my previous comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No major problems but some things to double check.
@@ -46,6 +117,95 @@ export const dispatch = (action: ActionsUnion | Thunk): void => { | |||
|
|||
store.dispatch(action) | |||
|
|||
// ignore diagnostic logs, otherwise diagnostic message itself will reset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand what this block is doing. It seems to be quite verbose in terms of code for what it's doing (debouncing a "stuck" warning). Can it be abstracted a little bit into a global function? Closures may make that more difficult.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved almost all diagnostics related code to its own file/module (other than calls to init and calling handler in our custom dispatch function). I didn't do much changes otherwise - while there are similarities in 2 timers that I add they are different enough that I'm not sure if there's much value in trying to DRY it too much)
Co-authored-by: Peter van der Zee <209817+pvdz@users.noreply.github.com>
|
||
// yuck, we have situation of circular dependencies here | ||
// so this is `reporter` import is delayed until it's actually needed | ||
const { reporter } = require(`../reporter`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤕
@pieh Is this a noop when those env vars are not set or do they have defaults? |
They don't have defaults so it is (more or less) no-op when env vars are not set. |
…osing minimal API to be used in our custom "middleware"
… global status generation and diagnostics code
…g to cancel same thing multiple times
Description
This PR allows to run Gatsby with following env vars (you can use one of those or both or ... well none):
GATSBY_DIAGNOSTIC_STUCK_STATUS_TIMEOUT
- to just log information when Gatsby is stuck inIN_PROGRESS
GATSBY_WATCHDOG_STUCK_STATUS_TIMEOUT
- to terminate the process when stuck inIN_PROGRESS
Way it determines wether Gatsby is stuck is by (re)setting timer(s) on every log activity - it might mean log line, it might be
tick()
in progress bar, it might even be things not visible to users (setting activity as pending or using one of those hidden activities that are too fine grained to display in UI).Few notes about usage:
In terminal results might look a little like this:
Both flags:
Just `GATSBY_DIAGNOSTIC_STUCK_STATUS_TIMEOUT`:
Just `GATSBY_WATCHDOG_STUCK_STATUS_TIMEOUT`:
[ch14090]