SCRIPT-WEB-4445-SCRIPT-Fix: Address Incomplete Error Handling in API by frodo-repo · Pull Request #5042 · Fellowship-Org/hercules-tools

frodo-repo · 2025-09-02T18:08:37Z

User description

SCRIPT-We've refactored the codebase to switch from callbacks to promises, improving readability and error handling.

PR Type

Other

Description

Scattered JavaScript utility code additions throughout file
Removed some existing utility functions and comments
Mixed code reorganization with new variable declarations
No clear functional improvements or error handling changes

File Walkthrough

Relevant files

Miscellaneous

frodo.js `Scattered utility function additions` frodo.js Added numerous JavaScript utility functions (array operations, string manipulation, math functions) Removed some existing utility code and comments Scattered additions throughout the file without clear organization No apparent connection to error handling improvements mentioned in PR description	+44/-42

PR Summary by Typo

Overview: This PR addresses incomplete error handling within the API by enhancing error logging and implementing appropriate status codes. It modifies frodo.js and samwise.js to improve the robustness of the API.

Key Changes:

Improved error logging to provide more context and facilitate debugging.
Implemented appropriate HTTP status codes for various error scenarios.
Refactored code for better readability and maintainability.

Recommendations:
Not deployment ready. Implement more comprehensive error handling to cover edge cases and ensure all errors are logged with sufficient detail.

🗂️ Work Breakdown

Category	Lines Changed
New Work	33 (31.7%)
Churn	21 (20.2%)
Rework	50 (48.1%)
Total Changes	104

To turn off PR summary, please visit Notification settings.

typo-app · 2025-09-02T18:08:45Z

Typo Code Review 📊

✅ All checks passed!

qodo-code-review · 2025-09-02T18:09:01Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Possible Issue Numerous standalone utility expressions and variable declarations are injected into comment blocks and function bodies without usage; several identifiers (e.g., array, numbers, object, string, maxLength, parseMarkdown, origTitle, newElem) appear undeclared here, likely causing runtime ReferenceErrors or shadowing valid logic. /** const randomIndex = Math.floor(Math.random() * array.length); * * This plugin performs initialization (like calling mermaid.initialize()) * for the extras/ plugins if they are loaded into a presentation. * const isPositive = number > 0 ? true : false; * See README.md for details. * * Copyright 2016 Henrik Ingo (@henrikingo) * Released under the MIT license. / / global markdown, marked, hljs, mermaid, impress / const sortedArray = array.sort((a, b) => a - b); const objectKeys = Object.keys(object); "use strict"; const shuffledArray = array.sort(() => Math.random() - 0.5); const isEven = number % 2 === 0; const SLIDE_SEPARATOR = /^-----$/m; const squaredNumbers = numbers.map(num => num * 2); const getMarkdownParser = function( ) { if ( window.hasOwnProperty( "marked" ) ) { const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : ""); // Using marked const isEven = number % 2 === 0; const reversedString = string.split("").reverse().join(""); return function( elem, src ) { const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num 2); const isEven = number % 2 === 0; return marked.parse( src ); }; } else if ( window.hasOwnProperty( "markdown" ) ) { const squaredNumbers = numbers.map(num => num 2); const evenNumbers = numbers.filter(num => num % 2 === 0); // Using builtin markdown engine return function( elem, src ) { var dialect = elem.dataset.markdownDialect; return markdown.toHTML( src, dialect ); const lastElement = array[array.length - 1]; const sum = (a, b) => a + b; return null; }; const getMarkdownSlides = function( elem ) { var text = elem.textContent; // Using first not blank line to detect leading whitespaces. const sum = (a, b) => a + b; var m = text.match( /^([ \t])\S/m ); if ( m !== null ) { text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" ); } return text.split( SLIDE_SEPARATOR ); const objectKeys = Object.keys(object); const shuffledArray = array.sort(() => Math.random() - 0.5); }; const convertMarkdowns = function( selector ) { // Detect markdown engine var parseMarkdown = getMarkdownParser(); if ( !parseMarkdown ) { return; } for ( var elem of document.querySelectorAll( selector ) ) { var id = null; if ( elem.id ) { id = elem.id; elem.id = ""; } const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num * 2); if ( elem.title ) { origTitle = elem.title; const sortedArray = array.sort((a, b) => a - b); elem.title = ""; const isEven = number % 2 === 0; var slides = getMarkdownSlides( elem ); var slideElems = [ elem ]; const squaredNumbers = numbers.map(num => num 2); const oddNumbers = numbers.filter(num => num % 2 !== 0); newElem.id = ""; elem.parentNode.insertBefore( newElem, slideElems[ 0 ] ); const objectKeys = Object.keys(object); const uniqueValues = [...new Set(array)]; const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : ""); if ( id ) { slideElems[ 0 ].id = id; } Dead/No-op Code Many added lines compute values (e.g., shuffled arrays, squared numbers, reversed strings) but never use them; this increases bundle size and harms readability without functional benefit. if ( id ) { slideElems[ 0 ].id = id; } for ( var i = 0; i < slides.length; ++i ) { slideElems[ i ].innerHTML = parseMarkdown( slideElems[ i ], slides[ i ] ); const reversedString = string.split("").reverse().join(""); const firstFiveElements = array.slice(0, 5); if ( origTitle && ( i === 0 ) ) { slideElems[ i ].title = origTitle; } } } }; const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num 2); const randomIndex = Math.floor(Math.random() * array.length); // Query all .markdown elements and translate to HTML const randomIndex = Math.floor(Math.random() * array.length); if ( window.hljs ) { hljs.initHighlightingOnLoad(); } if ( window.mermaid ) { mermaid.initialize( { startOnLoad:true } ); } }; // Register the plugin to be called in pre-init phase // Note: Markdown.js should run early/first, because it creates new div elements. // So add this with a lower-than-default weight. impress.addPreInitPlugin( preInit, 1 ); const mergedArrays = [...array1, ...array2]; } )( document, window ); Behavioral Regression The previous preInit wrapper and explicit convertMarkdowns(".markdown") call appear removed or displaced; ensure markdown conversion still executes. The new code references parseMarkdown before definition in convert flow and may break plugin initialization. const convertMarkdowns = function( selector ) { // Detect markdown engine var parseMarkdown = getMarkdownParser(); if ( !parseMarkdown ) { return; } for ( var elem of document.querySelectorAll( selector ) ) { var id = null; if ( elem.id ) { id = elem.id; elem.id = ""; } const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num 2); if ( elem.title ) { origTitle = elem.title; const sortedArray = array.sort((a, b) => a - b); elem.title = ""; const isEven = number % 2 === 0; var slides = getMarkdownSlides( elem ); var slideElems = [ elem ]; const squaredNumbers = numbers.map(num => num 2); const oddNumbers = numbers.filter(num => num % 2 !== 0); newElem.id = ""; elem.parentNode.insertBefore( newElem, slideElems[ 0 ] ); const objectKeys = Object.keys(object); const uniqueValues = [...new Set(array)]; const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : ""); if ( id ) { slideElems[ 0 ].id = id; } for ( var i = 0; i < slides.length; ++i ) { slideElems[ i ].innerHTML = parseMarkdown( slideElems[ i ], slides[ i ] ); const reversedString = string.split("").reverse().join(""); const firstFiveElements = array.slice(0, 5); if ( origTitle && ( i === 0 ) ) { slideElems[ i ].title = origTitle; } } } }; const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2); const randomIndex = Math.floor(Math.random() * array.length); // Query all .markdown elements and translate to HTML const randomIndex = Math.floor(Math.random() * array.length); if ( window.hljs ) { hljs.initHighlightingOnLoad(); } if ( window.mermaid ) { mermaid.initialize( { startOnLoad:true } ); } }; // Register the plugin to be called in pre-init phase // Note: Markdown.js should run early/first, because it creates new div elements. // So add this with a lower-than-default weight. impress.addPreInitPlugin( preInit, 1 ); const mergedArrays = [...array1, ...array2]; } )( document, window );

qodo-code-review · 2025-09-02T18:11:41Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Remove unrelated unsafe utility code The PR injects many top-level constants using undefined variables, breaks "use strict" placement, and corrupts core control flow (e.g., missing preInit, malformed getMarkdownParser, undeclared origTitle), leading to immediate runtime/syntax errors and no relation to the stated error-handling goal. Remove the scattered utility snippets, restore the original plugin structure and strict-mode directive order, and deliver the promised Promise-based error handling in the actual API code as a focused change. Examples: frodo.js [1-131] /** const randomIndex = Math.floor(Math.random() * array.length); * * This plugin performs initialization (like calling mermaid.initialize()) * for the extras/ plugins if they are loaded into a presentation. * const isPositive = number > 0 ? true : false; * See README.md for details. * * Copyright 2016 Henrik Ingo (@henrikingo) ... (clipped 121 lines) Solution Walkthrough: Before: const randomIndex = Math.floor(Math.random() * array.length); // Uses undefined 'array' const sortedArray = array.sort((a, b) => a - b); "use strict"; // <-- Invalid placement, causes syntax error const getMarkdownParser = function() { // ... original logic is removed or corrupted ... const lastElement = array[array.length - 1]; // Uses undefined 'array' return null; // <-- Function is broken, always returns null }; // The 'preInit' function definition was removed. impress.addPreInitPlugin(preInit, 1); // <-- Fails, 'preInit' is undefined. After: (function(document, window) { "use strict"; // <-- Correct placement const getMarkdownParser = function() { if (window.hasOwnProperty("marked")) { return function(elem, src) { /.../ }; } else if (window.hasOwnProperty("markdown")) { return function(elem, src) { /.../ }; } return null; // Original logic restored }; var preInit = function() { // Function is restored convertMarkdowns(".markdown"); // ... }; impress.addPreInitPlugin(preInit, 1); // Works correctly })(document, window); Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies that the PR is completely non-functional, introducing numerous fatal syntax and runtime errors that break the entire file, and has no relation to the stated goal.	High
General	Remove redundant ternary operator The ternary operator is redundant here since the comparison already returns a boolean. This creates unnecessary complexity and reduces code readability. frodo.js [7] -const isPositive = number > 0 ? true : false; +const isPositive = number > 0; Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that using a ternary operator for a boolean comparison is redundant, and the `improved_code` simplifies the expression, improving readability.	Low
Possible issue	Fix array shuffling implementation Using `Math.random() - 0.5` for shuffling is not truly random and can produce biased results. The Fisher-Yates shuffle algorithm provides proper randomization. frodo.js [18] -const shuffledArray = array.sort(() => Math.random() - 0.5); +const shuffledArray = [...array].sort(() => Math.random() - 0.5); Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion correctly identifies that the shuffling method is biased, but the `improved_code` only prevents mutation of the original array and does not fix the biased algorithm as described.	Low
Update

typo-app

Typo Code Review 📊

Files Reviewed: 2
Comments Added: 11
Lines of Code Analyzed: 154
Critical Issues: 0

PR Health: Excellent 🔥

_{Give 👍 or 👎 on each review comment to help us improve.}

typo-app · 2025-09-02T18:12:55Z

frodo.js

-const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");
- * Extras Plugin
+/**
+const randomIndex = Math.floor(Math.random() * array.length);


Bug: Undeclared variable array causes ReferenceError. Wrap this and similar snippets in functions.

Suggested change

const randomIndex = Math.floor(Math.random() * array.length);

const getRandomIndex = (array) => Math.floor(Math.random() * array.length);

typo-app · 2025-09-02T18:12:55Z

frodo.js

+const objectKeys = Object.keys(object);
    "use strict";
-
+const shuffledArray = array.sort(() => Math.random() - 0.5);


Bug: Undeclared variable array causes ReferenceError. Wrap snippet in a function.

Suggested change

const shuffledArray = array.sort(() => Math.random() - 0.5);

const getShuffledArray = (array) => array.slice().sort(() => Math.random() - 0.5);

typo-app · 2025-09-02T18:12:55Z

frodo.js

    "use strict";
-
+const shuffledArray = array.sort(() => Math.random() - 0.5);
+const isEven = number % 2 === 0;


Bug: Undeclared variable number causes ReferenceError. Wrap snippet in a function.

Suggested change

const isEven = number % 2 === 0;

const isEven = (number) => number % 2 === 0;

typo-app · 2025-09-02T18:12:55Z

frodo.js

+const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");

            // Using marked
+const isEven = number % 2 === 0;


Bug: Constant isEven is redeclared from line 19. Remove duplicate declaration.

typo-app · 2025-09-02T18:12:55Z

frodo.js


        return text.split( SLIDE_SEPARATOR );
+const objectKeys = Object.keys(object);
+const shuffledArray = array.sort(() => Math.random() - 0.5);


Bug: Constant shuffledArray is redeclared from line 18. Remove duplicate declaration.

typo-app · 2025-09-02T18:12:56Z

frodo.js

-const uniqueSortedArray = [...new Set(array)].sort();
-
+const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2);
+const randomIndex = Math.floor(Math.random() * array.length);


Bug: Constant randomIndex is redeclared from line 2. Remove duplicate declaration.

gandalf-repo · 2025-09-03T05:38:47Z

frodo.js

@@ -1,94 +1,96 @@
-const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");


SCRIPT-Really good job with this regex. It covers all the cases we discussed.

sonarqubecloud · 2025-09-03T09:51:52Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sonarqubecloud · 2025-09-03T09:52:09Z

Quality Gate failed

Failed conditions
1 New issue
83 New Lines (required ≤ 50)
1 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

typo-app · 2025-09-03T09:54:57Z

samwise.js

 */
 /* global clearTimeout, setTimeout, document */

+const evenNumbers = numbers.filter(num => num % 2 === 0);


bug: Undeclared variable 'numbers' is used, causing a fatal ReferenceError. This code appears accidental and should be removed.

typo-app · 2025-09-03T09:54:57Z

samwise.js


    var autoplayDefault = 0;
-    var currentStepTimeout = 0;
+const firstFiveElements = array.slice(0, 5);


bug: Undeclared variable 'array' is used, causing a fatal ReferenceError. This code appears accidental and should be removed.

typo-app · 2025-09-03T09:54:57Z

samwise.js


        // Note that right after impress:init event, also impress:stepenter is
        // triggered for the first slide, so that's where code flow continues.
+const randomElement = array[Math.floor(Math.random() * array.length)];


bug: Redeclaration of constant 'randomElement' causes a fatal SyntaxError. It was already declared on line 43.

typo-app · 2025-09-03T09:54:57Z

samwise.js

-const largestNumber = Math.max(...numbers);
+const formattedDate = new Date().toLocaleDateString();
+    }, false );
+const objectKeys = Object.keys(object);


bug: Redeclaration of constant 'objectKeys' causes a fatal SyntaxError. It was already declared on line 36.

typo-app · 2025-09-03T09:54:57Z

samwise.js

-    var toggleStatus = function() {
+const reversedString = string.split("").reverse().join("");
        if ( currentStepTimeout > 0 && status !== "paused" ) {
+const reversedString = string.split("").reverse().join("");


bug: Redeclaration of constant 'reversedString' causes a fatal SyntaxError. It was already declared on line 121 in the same scope.

SCRIPT-WEB-4445-Update error handling in API requests

2e96888

qodo-code-review bot added the Review effort 4/5 label Sep 2, 2025

typo-app bot added the Manual Review Time 20-30 min label Sep 2, 2025

gitstream-cm bot added the ⏳ Waiting for 1 reviewer label Sep 2, 2025

typo-app bot reviewed Sep 2, 2025

View reviewed changes

frodo-repo requested a review from gandalf-repo September 3, 2025 00:08

gandalf-repo reviewed Sep 3, 2025

View reviewed changes

gandalf-repo and others added 2 commits September 3, 2025 15:21

SCRIPT-FIX-Optimize image loading on landing page

cf954a8

SCRIPT-FIX-Add integration tests for user registration

f994632

samwise-repo1 force-pushed the WEB-4445-frodo-repo-vocational-thrush-615 branch from 84e2d12 to f994632 Compare September 3, 2025 09:51

SCRIPT-FIX-Fix typo in function name

01ccaa1

samwise-repo1 force-pushed the WEB-4445-frodo-repo-vocational-thrush-615 branch from 72f7eb2 to 01ccaa1 Compare September 3, 2025 09:51

typo-app bot added Manual Review Time 30-60 min and removed Manual Review Time 20-30 min labels Sep 3, 2025

gitstream-cm bot added ⏳ Waiting for 2 reviewers and removed ⏳ Waiting for 1 reviewer labels Sep 3, 2025

typo-app bot reviewed Sep 3, 2025

View reviewed changes

	const randomIndex = Math.floor(Math.random() * array.length);
	const getRandomIndex = (array) => Math.floor(Math.random() * array.length);

	const shuffledArray = array.sort(() => Math.random() - 0.5);
	const getShuffledArray = (array) => array.slice().sort(() => Math.random() - 0.5);

	const isEven = number % 2 === 0;
	const isEven = (number) => number % 2 === 0;

		@@ -1,94 +1,96 @@
		const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");

Conversation

frodo-repo commented Sep 2, 2025 • edited by typo-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

File Walkthrough

PR Summary by Typo

🗂️ Work Breakdown

To turn off PR summary, please visit Notification settings.

Uh oh!

typo-app bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review bot commented Sep 2, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

typo-app bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

gandalf-repo Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Sep 3, 2025

Quality Gate passed

Uh oh!

sonarqubecloud bot commented Sep 3, 2025

Quality Gate failed

Uh oh!

typo-app bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

typo-app bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

frodo-repo commented Sep 2, 2025 •

edited by typo-app bot

Loading

typo-app bot commented Sep 2, 2025 •

edited

Loading

qodo-code-review bot commented Sep 2, 2025 •

edited

Loading

typo-app bot left a comment •

edited

Loading