🔨 correctly initialize bugsnag in SiteBaker v2 #4005
Conversation
db/model/Gdoc/GdocBase.ts
Outdated
| @@ -750,6 +751,8 @@ export class GdocBase implements OwidGdocBaseInterface { | |||
| const chartIdsBySlug = await mapSlugsToIds(knex) | |||
| const publishedExplorersBySlug = | |||
| await db.getPublishedExplorersBySlug(knex) | |||
| const redirects = await getRedirectsFromDb(knex) | |||
| const resolvedRedirects = resolveRedirects(redirects) | |||
There was a problem hiding this comment.
It seems we call validate on every gdoc in a loop when baking, which means we'd make the same query many times. We could move the logic to a separate function and make the query (and resolve redirects) only once.
Similarly for other DB queries we make here, but that is out of scope.
There was a problem hiding this comment.
I agree, though refactoring gdocs to validate in bulk would probably be best done in a separate PR.
As is, I've decided just to not do this validation - we've never changed an explorer to point to another explorer (except with coronavirus-data-explorer) so I think if it comes up again, we can just update the (likely fewer than 10) gdocs that are affected.
db/model/Redirect.ts
Outdated
| @@ -16,6 +16,29 @@ export async function getRedirects( | |||
| ) | |||
| } | |||
|
|
|||
| /** | |||
| * Takes a list of redirects and returns a map of source -> target | |||
| * such that if you have [A -> B, B -> C], the map will be { A -> C, B -> C } | |||
There was a problem hiding this comment.
I thought there were no redirect chains in the DB. We have a validation that prevents adding new chains, and if there are any existing, it would be better if we resolved/changed them in the DB instead of always resolving them here.
There was a problem hiding this comment.
True! I had some leftover testing data in my redirects table it seems. Not that it matters now that I've removed the explorer validation 🙂
|
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2024-10-08 22:35:26 UTC |
Fixes #3019
I wasn't able to re-open the original PR for this branch because I rebased the branch after the PR had been closed by Stalebot.
tl;dr: - it was tricky to robustly fix all the errors and warnings currently in prod with code. We'll be able fix some of them by updating the data and can ignore the rest in bugsnag.
In the other PR, I had proposed 3 ways to fix the existing errors that initializing bugsnag correctly would begin to expose:
Each had a sort of problem.
For 1: I added code that considers redirects, but the
coronavirus-data-explorerredirect is hardcoded in thebaker/redirects.tsfile and can't be easily imported to use in gdocs validation. We could ignore all these errors, or update and republish the articles that use the old URL.For 2: we don't really have a good way to exclude a certain class of error/warning from Bugsnag because we don't have error codes. We can't ignore all warnings, because there are some warnings that we do want to be notified about such as a linked gdoc no longer existing, which we can't upgrade to errors because that prevents a gdoc from being published.
For 3: all validation errors for a gdoc were being serialized together and logged, which would mean that an ignored error might resurface if its gdoc acquired another type of error. I've fixed this by logging each error/warning to bugsnag individually.