fix(check_redirects): make the checks fail fast #7226

OnkarRuikar · 2022-09-24T07:21:40Z

Summary

Wrapping original error and failing at the end, after all the checks are done, makes it hard to figure out what went wrong.
Refer mdn/translated-content#8171 (comment)

Problem

The check run https://github.com/mdn/translated-content/actions/runs/3114446745/jobs/5050295335 failed with following log:

Run yarn content validate-redirects --strict
yarn run v1.22.19
$ env-cmd --silent cross-env CONTENT_ROOT=files yari-tool validate-redirects --strict
Skipping: non en-us locale: /en-US/docs/Secciones_y_contornos_de_un_documento_HTML5	/es/docs/Sections_and_Outlines_of_an_HTML5_document
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Getting_started_with_WebGL/Commencer_avec_le_WebGL	/fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Tutorial/Getting_started_with_WebGL/Commencer_avec_le_WebGL	/fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL	/fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL	/fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/video	/es/docs/Web/HTML/Elemento/video
Skipping: non en-us locale: /en-US/docs/zh-n/JavaScript/Reference/Global_Objects/String/quote	/zh-CN/docs/Web/JavaScript/Reference/Global_Objects/String/quote
info: ✓ redirects for en-us looking good!
Skipping: non es locale: /es/docs/Web/JavaScript/Guide/Obsolete_Pages/The_Employee_Example	/en-US/docs/Web/JavaScript/Guide/Details_of_the_Object_Model
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear	/es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly	/es/docs/WebAssembly/JavaScript_interface
info: _redirects.txt for es is causing issues: Error: Invalid redirect for /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear -> /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features or /es/docs/Web/JavaScript/Reference/Global_Objects/null -> /es/docs/Web/JavaScript/Reference/Operators/null
info: ✓ redirects for fr looking good!
info: ✓ redirects for ja looking good!
info: ✓ redirects for ko looking good!
info: ✓ redirects for pt-br looking good!
info: ✓ redirects for ru looking good!
info: ✓ redirects for zh-cn looking good!
Skipping: non zh-tw locale: /zh-TW/docs/Web/API/Detecting_device_orientation	/en-US/docs/Web/Events/Detecting_device_orientation
Skipping: non zh-tw locale: /zh-TW/docs/WebAPI/Detecting_device_orientation	/en-US/docs/Web/Events/Detecting_device_orientation
info: ✓ redirects for zh-tw looking good!

error: 🔥 Errors loading redirects 🔥

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

There are no colors in GitHub's run logs.
The log before the error 🔥 Errors loading redirects 🔥 message said all went well: info: ✓ redirects for zh-tw looking good!.
So the PR author failed to figure out what actually went wrong:

Solution

Fail fast: we do not have to continue checking remaining locales if one fails.
Fail on the spot: delaying the reporting makes it hard to figure out what and where is went wrong.

Screenshots

Before

❯ yarn tool validate-redirects --strict
yarn run v1.22.19
$ ts-node tool/cli.ts validate-redirects --strict
Skipping: non en-us locale: /en-US/docs/Secciones_y_contornos_de_un_documento_HTML5     /es/docs/Sections_and_Outlines_of_an_HTML5_document
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Getting_started_with_WebGL/Commencer_avec_le_WebGL    /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Tutorial/Getting_started_with_WebGL/Commencer_avec_le_WebGL   /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL    /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL        /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/video   /es/docs/Web/HTML/Elemento/video
Skipping: non en-us locale: /en-US/docs/zh-n/JavaScript/Reference/Global_Objects/String/quote   /zh-CN/docs/Web/JavaScript/Reference/Global_Objects/String/quote
info: ✓ redirects for en-us looking good!
Skipping: non es locale: /es/docs/Web/JavaScript/Guide/Obsolete_Pages/The_Employee_Example      /en-US/docs/Web/JavaScript/Guide/Details_of_the_Object_Model
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear        /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly  /es/docs/WebAssembly/JavaScript_interface
info: _redirects.txt for es is causing issues: Error: Invalid redirect for /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear -> /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features or /es/docs/Web/JavaScript/Reference/Global_Objects/null -> /es/docs/Web/JavaScript/Reference/Operators/null
info: ✓ redirects for fr looking good!
info: ✓ redirects for ja looking good!
info: ✓ redirects for ko looking good!
info: ✓ redirects for pt-br looking good!
info: ✓ redirects for ru looking good!
info: ✓ redirects for zh-cn looking good!
Skipping: non zh-tw locale: /zh-TW/docs/Web/API/Detecting_device_orientation    /en-US/docs/Web/Events/Detecting_device_orientation
Skipping: non zh-tw locale: /zh-TW/docs/WebAPI/Detecting_device_orientation     /en-US/docs/Web/Events/Detecting_device_orientation
info: ✓ redirects for zh-tw looking good!

error: 🔥 Errors loading redirects 🔥

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

After

❯ yarn tool validate-redirects --strict
yarn run v1.22.19
$ ts-node tool/cli.ts validate-redirects --strict
Skipping: non en-us locale: /en-US/docs/Secciones_y_contornos_de_un_documento_HTML5     /es/docs/Sections_and_Outlines_of_an_HTML5_document
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Getting_started_with_WebGL/Commencer_avec_le_WebGL    /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Tutorial/Getting_started_with_WebGL/Commencer_avec_le_WebGL   /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL    /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL        /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/video   /es/docs/Web/HTML/Elemento/video
Skipping: non en-us locale: /en-US/docs/zh-n/JavaScript/Reference/Global_Objects/String/quote   /zh-CN/docs/Web/JavaScript/Reference/Global_Objects/String/quote
info: ✓ redirects for en-us looking good!
Skipping: non es locale: /es/docs/Web/JavaScript/Guide/Obsolete_Pages/The_Employee_Example      /en-US/docs/Web/JavaScript/Guide/Details_of_the_Object_Model
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear        /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly  /es/docs/WebAssembly/JavaScript_interface

error: _redirects.txt for es is causing issues: Error: Invalid redirect for /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear -> /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features or /es/docs/Web/JavaScript/Reference/Global_Objects/null -> /es/docs/Web/JavaScript/Reference/Operators/null


error: 🔥 Errors loading redirects 🔥

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

How did you test this change?

In terminal using command yarn tool validate-redirects --strict

OnkarRuikar · 2022-09-30T01:07:39Z

Ping @caugner

caugner

If the checks fail fast, doesn't this mean that only the first error will be displayed?

Alternatively, could we make sure that all output is prefixed with either info: or error: , and additionally prefix the error messages with ❌ to make them easier to discover? (Likewise, skipping could have ➖.)

OnkarRuikar · 2022-10-21T04:02:10Z

If the checks fail fast, doesn't this mean that only the first error will be displayed?

If a locale has error(s) it'll be processed but remaining locales after it won't be processed.
This shouldn't matter as hardly any PR works on files of multiple locales, because code owners/reviewers are different for each locale. Major change like rename/delete files certainly happens separately for each locale.

We shouldn't keep the feature (of gathering errors) for that very rare PR. Unless I am missing any historical context, in such case, the contributor will not hesitate to fix redirection errors one by one for each locale.

caugner

Alright, let's stick with throwing, but then we can throw the specific error message directly.

tool/cli.ts

OnkarRuikar · 2022-10-30T02:30:04Z

After applying the changes:

❯ yarn tool validate-redirects --strict
yarn run v1.22.19
$ ts-node tool/cli.ts validate-redirects --strict
Skipping: non en-us locale: /en-US/docs/Secciones_y_contornos_de_un_documento_HTML5    /es/docs/Sections_and_Outlines_of_an_HTML5_document
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Getting_started_with_WebGL/Commencer_avec_le_WebGL      /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/Web/API/WebGL_API/Tutorial/Getting_started_with_WebGL/Commencer_avec_le_WebGL     /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
avec_le_WebGL en/fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGLebGL/Commencer_
Skipping: non en-us locale: /en-US/docs/WebGL/Getting_started_with_WebGL/Commencer_avec_le_WebGL       /fr/docs/Web/API/WebGL_API/Tutorial/Commencer_avec_WebGL
Skipping: non en-us locale: /en-US/docs/video   /es/docs/Web/HTML/Elemento/video
Skipping: non en-us locale: /en-US/docs/zh-n/JavaScript/Reference/Global_Objects/String/quote     /zh-CN/docs/Web/JavaScript/Reference/Global_Objects/String/quote
info: ✓ redirects for en-us looking good!
Skipping: non es locale: /es/docs/Web/JavaScript/Guide/Obsolete_Pages/The_Employee_Example      /en-US/docs/Web/JavaScript/Guide/Details_of_the_Object_Model
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear        /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features
removing orphaned redirect (from exists): /es/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly  /es/docs/WebAssembly/JavaScript_interface

error: _redirects.txt for es is causing issues: Error: Invalid redirect for /es/docs/Web/JavaScript/Reference/Global_Objects/WeakMap/clear -> /es/docs/conflicting/Web/JavaScript/Reference/Deprecated_and_obsolete_features or /es/docs/AJAX -> /es/docs/Web/Guide/AJAX

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

OnkarRuikar · 2022-11-04T04:56:12Z

Ping @caugner

OnkarRuikar force-pushed the fix-check_redirects branch from 7dfb281 to bfd8e7e Compare October 3, 2022 07:46

schalkneethling requested a review from fiji-flo October 3, 2022 11:26

schalkneethling assigned fiji-flo Oct 3, 2022

schalkneethling added 🧑‍🤝‍🧑 community contributions by our wonderful community 🚉 platform keeping the platform healthy labels Oct 3, 2022

OnkarRuikar force-pushed the fix-check_redirects branch from bfd8e7e to 3f433c8 Compare October 15, 2022 09:46

caugner reviewed Oct 20, 2022

View reviewed changes

caugner removed the request for review from fiji-flo October 20, 2022 18:47

caugner unassigned fiji-flo Oct 20, 2022

caugner requested changes Oct 24, 2022

View reviewed changes

tool/cli.ts Outdated Show resolved Hide resolved

tool/cli.ts Outdated Show resolved Hide resolved

OnkarRuikar force-pushed the fix-check_redirects branch from 53c4c1a to d4da6eb Compare October 30, 2022 02:26

OnkarRuikar added 2 commits November 4, 2022 10:19

fix(check_redirects): make the checks fail fast

145c156

Directly throw with the message without logging

f8d58d4

OnkarRuikar force-pushed the fix-check_redirects branch from d4da6eb to f8d58d4 Compare November 4, 2022 04:49

caugner approved these changes Nov 7, 2022

View reviewed changes

caugner merged commit 33c907f into mdn:main Nov 7, 2022

OnkarRuikar deleted the fix-check_redirects branch November 8, 2022 02:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(check_redirects): make the checks fail fast #7226

fix(check_redirects): make the checks fail fast #7226

OnkarRuikar commented Sep 24, 2022

OnkarRuikar commented Sep 30, 2022

caugner left a comment

OnkarRuikar commented Oct 21, 2022

caugner left a comment

OnkarRuikar commented Oct 30, 2022

OnkarRuikar commented Nov 4, 2022

fix(check_redirects): make the checks fail fast #7226

fix(check_redirects): make the checks fail fast #7226

Conversation

OnkarRuikar commented Sep 24, 2022

Summary

Problem

Solution

Screenshots

Before

After

How did you test this change?

OnkarRuikar commented Sep 30, 2022

caugner left a comment

Choose a reason for hiding this comment

OnkarRuikar commented Oct 21, 2022

caugner left a comment

Choose a reason for hiding this comment

OnkarRuikar commented Oct 30, 2022

OnkarRuikar commented Nov 4, 2022