refs https://github.com/TryGhost/Ghost/pull/13716
refs https://github.com/actions/setup-node/issues/317#issuecomment-929694556
- the `setup-node` GitHub Action seems to use a shell command to get the
cache path, but these are colorised when `FORCE_COLOR` is enabled
- this causes the Action to fail to read the path correctly
- the comment referenced above suggests to remove `FORCE_COLOR` but it's
nice to have colored output for our tests
- instead, I'm disabling the environment variable on the `setup-node`
action so it still works
- I've tested with the referenced PR and this unblocks dependency caching 🎉
refs https://github.com/TryGhost/Toolbox/issues/139
- The v2 and v3 redirects APIs are unofficial and should not be used by anyone in production. There's no good reason to maintain expensive to run test suites for old unofficial APIs.
- The test cases in canary suite covers the functionality of redirects enough to be sure they work as expeted
refs https://github.com/TryGhost/Toolbox/issues/138
- These tests still rely on the frontend to be present. Needs further investigation to remove "frontend: true" flag - it slows down test runs!
refs https://github.com/TryGhost/Toolbox/issues/138
- Final batch of the refactor to async/await syntax. Doing these refactors before modifying "testUtils.startGhost" everywhere to boot only with the backend
refs https://github.com/TryGhost/Toolbox/issues/138
- This is a continuation of a bigger refactor to use async/await syntax before migrating "startGhost" methods to only use backend boot
- Removed a little bit of dead code (like admin user creation) which should speed up test execution too!
- Refactored user variables to be declared closer to their usecases instead of being high up in a global scope - variables shoul not live that far apart from the code that uses them
refs https://github.com/TryGhost/Toolbox/issues/138
- First batch of the refactor to async/await syntax. Next one will cover the rest. Doing these refactors before modifying "testUtils.startGhost" everywhere to boot only with the backend
refs https://github.com/TryGhost/Toolbox/issues/138
- There is no good reason to keep this extra variable around just call "stop" in couple very specific cases. Even for those cases, there's `testUtils.stopGhost` method which achieves the same without additional variable to track.
refs https://github.com/TryGhost/Toolbox/issues/138
- Having the "ghost" alias only added cognitive load when reading through the test code and didn't provide any additional value. Removed the pattern to keep things simpler and more explicit
refs https://github.com/TryGhost/Toolbox/issues/138
- Using asycn/await syntax is way more readable and allows to identify further reusable patterns in test initialization. This refactor also served as an exploreation around how the code looks like at this point
- When we handle errors in Ghost, we are supposed to use a pattern of supplying 3 messages:
- message: what went wrong
- context: details about why how or where the error happened
- help: where the user can go to get help with this error
- We do this in many places and our JSON error handler and CLI error logging tools are designed to output this extra information
- However, stack traces, which start with message as the first line and then output the stack are totally missing this
- By injecting the additional messages into the stack once an error has been "ghostified" we should get clearer messages everywhere
Notes:
- I've additionally injected a "Stack Trace:" line that makes it easier to read the error vs the stack
- This code looks a little weird because the lines are inserted backwards, but that allows us to always to the insert at position 1 as per the comment,
so we don't have to keep track of whether we already injected something or not
refs: 2af9e2e12
- This new HTMLErrorRenderer is borrowed heavily from finalHandler
- This is the module that express uses to render errors if there is no custom errorhandler
- It just renders a really simple html page wrapping err.stack in a <pre>
- This results in a nicely formatted, but unstyled error page
- I also updated BasicErrorRenderer to use the same res.statusCode + err.stack pattern rather than err.message
Note: This error renderer is _only_ used for renderering errors on the `/ghost/` route
- In almost all cases, errors here are rendered by Ember
- The only error that can be rendered here is a missing template error see: 2af9e2e12
- If the admin templates default.html or default-prod.html are missing, don't throw a 500
- Instead throw a well considered 400 error with extra help for what to do to fix it
- Reduced our maintenance middleware code down to the bare minimum!
- We have an old maintenance middleware in place to handle when a site is forcibly put into maintenance mode, or the urlService hasn't finished booting
- This maintenance middleware was mounted on every sub app, instead of globally for reasons I no longer remember
- Recently, we introduced a new, static version of maintenence middleware to show during the boot process so we can get the server started earlier & not drop requests
- This version has its own HTML template and doesn't depend on any of Ghost's error rendering code
- To simplify and help with decoupling, this commit merges the two middleware, so that the new independent & static middleware renders its template for any one of the 3 possible maintenance modes
- It only needs to exist in the top level app 🙌
TODO: move the maintenance middleware to its own file/package so it's not part of the app.js as that is weird
- throughout the theme activation flow there are several missing awaits and necessary async keywords
- we should be waiting on these processes, not letting them complete indeterministically
refs bb47b9e327
- EACCESS error was previously caught to stop the boot process from failing with perms errors
- For clearFiless, we do not care if these files cannot be removed. Refactored to use allSettled which means we don't do them in sequence + can ignore the outcome
- For minifiy, this is now a legit error, however we don't need the activate method to fail for an EACCES error, we just need an error to be shown (I think)
refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings
- It's a step to making the module follow class+DI pattern before fully extracting it into an external libarary
- Reminder, doing in Ghost repo instead of substituting big chunks all at once to have clear history of how the service evolved prior to the extraction into external lib!
refs https://github.com/TryGhost/Team/issues/1232
- Only require the right css rules depending on the `card_assets` configuration
- 🐛 removed `kg-width-wide` and `kg-width-full` from being considered as card asset, as they should always be defined in themes (it was only a problem in v4.11.x)
- Card asset reloading was incorrectly only happening if the API version changed 🙈
- In addition, having an init function was redundant, as theme activation happens on boot
- This meant that the card assets were being generated twice on boot
- Instead, we now only generate them on theme activation, which covers the boot case and simplifies all the logic
- Currently it's assumed that public files are 100% static
- With card assets, we're using it for files that are partially static, but can change between reboots and theme changes
- We already have a system for managing cache busting across theme changes and restarts - the ?v= key that is added via the asset helper
- This was already in place and used, but servePublicFile's internal cache didn't honor this key, and cached for the lifetime of boot
- This small change means that if a ?v= query param is present on a request for a public file, that we pay attention to it. Else we cache as before
no issue
When switching the oembed service to async/await the error handling was not correctly refactored. `this.errorHandler(url)` was returning a curried function so it could be used as `.catch(this.errorHandler(url))` but that's not how it's being used after the async/await change meaning we were returning a function rather than the result of that function.
- `this.errorHandler(url)` is now only used in one place where `url` is available so removed the method and moved the body of the curried function inline into the `catch` handler
- added a message to the logged error so it's more clear what the log refers to
refs https://github.com/TryGhost/Toolbox/issues/135
- Looking closer into the reason why the test was failing without "forceStart" revealed that the server only start was overoptimized - "initServicesForFrontend" should be a part of a backend as those are generic theme services.
no issue
- Having rewire here doens't do any difference and should not be used if absolutely needed. Usually using rewire gives a code "smell" so there's some sort of coupling that's going on and probably has to be addressed first
refs https://github.com/TryGhost/Toolbox/issues/135
- The reason the test **settings** test was failing when the force start flag was removed in the **custom themes** was the bridge! The bridge was trying to execute function on the frontend when the boot was done without initializing the frontend. The setting test was changing locale and the timezone which triggered events calling up on frontend components - we clearly don't want to do this when the instance is booted without the frontend
- To make event initialization conditional moved it to the "init". This way the event listeners are only set up when we boot with the "frontend" flag set to true
