feat: add integration tests & refactor scripts to runners #4396
Conversation
Co-authored-by: Chan <[email protected]> Co-authored-by: akshatnema <[email protected]> Co-authored-by: Akshat Nema <[email protected]>
akshatnema
requested review from
Mayaleeeee,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
WalkthroughAdds standardized runners for build scripts, introduces a CustomError system with structured logging, refactors multiple scripts to export functions and use centralized error handling, updates tests accordingly, and revises tooling configs (ESLint, Jest, package.json). Minor type tweaks and doc formatting are included. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as CLI/CI
participant Runner as Runner (npm/runners/*)
participant Script as Script (scripts/*)
participant Helpers as Helpers (logger, read/write)
participant Err as CustomError
Dev->>Runner: tsx npm/runners/...-runner.ts
alt NODE_ENV !== 'test'
Runner->>Script: call exported function(...)
Script->>Script: validate inputs/env
Script->>Helpers: read/write, fetch, transform
Helpers-->>Script: results or throw
Script-->>Runner: success
Runner-->>Dev: exit 0
else Test mode
Runner-->>Dev: skip auto-exec (log info)
end
rect rgba(255,235,238,0.6)
note over Script,Helpers: Error path
Script--x Err: throw (wrap via CustomError)
Runner->>Helpers: logger.error("...", CustomError)
Runner-->>Dev: exit 1 (on top-level failure)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
asyncapi-bot
requested review from
CBID2,
bandantonio,
devilkiller-ag and
vishvamsinh28
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4396 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 21 -1
Lines 778 828 +50
Branches 144 165 +21
=========================================
+ Hits 778 828 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4396--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
scripts/index.ts (2)
39-49: Year detection is too permissive; may pick “2023.json” as a yearparseFloat allows strings like "2023.json", which could break downstream when year is expected to be "2023". Filter using a strict 4-digit regex and directories.
- const yearsList = fs - .readdirSync(financeDir) - // filter out any files that are not numbers - .filter((file) => { - return !Number.isNaN(parseFloat(file)); - }) - // sort the years in descending order - .sort((a, b) => { - return parseFloat(b) - parseFloat(a); - }); + const yearsList = fs + .readdirSync(financeDir, { withFileTypes: true }) + // directories named as a 4-digit year + .filter((d) => d.isDirectory() && /^\d{4}$/.test(d.name)) + .map((d) => d.name) + .sort((a, b) => Number(b) - Number(a));Also applies to: 51-56
68-68: Avoid top-level side effects; gate executionWith runners in place, importing this module will auto-run start(). Gate it to only run when executed directly to prevent double builds.
-export { start }; - -start(); +export { start }; + +// Run only when executed directly, not when imported by runners/tests. +if (process.argv[1] === currentFilePath) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + start(); +}To confirm no other entrypoint already calls start():
#!/bin/bash rg -nC2 -e '\bstart\s*\(' -e "from '.*scripts/index" -e 'import .*scripts/index'scripts/build-post-list.ts (2)
222-224: Respect the resultObj parameter instead of the module-global finalResult.Using the global here couples walkDirectories to a single accumulator and hampers reuse/testing.
- addItem(details, finalResult); + addItem(details, resultObj); ... - addItem(details, finalResult); + addItem(details, resultObj);Also applies to: 273-274
16-23: Eliminate mutable module-level state (finalResult/specWeight).Keeping state at module scope risks cross-run pollution (e.g., multiple runner invocations or tests). Initialize per-call and pass explicitly.
-let specWeight = 100; -const finalResult: Result = { - docs: [], - blog: [], - about: [], - docsTree: {} -}; +// moved inside buildPostListAnd inside buildPostList:
export async function buildPostList( postDirectories: string[][], basePath: string, writeFilePath: string ): Promise<void> { try { + let specWeight = 100; + const finalResult: Result = { docs: [], blog: [], about: [], docsTree: {} };Also applies to: 312-359
scripts/compose.ts (2)
44-47: Use a valid ISO timestamp (current format is incorrect).
YYYY-MM-DDTh:mm:ssZmixes a literal T with 12‑hourh. UsetoISOString()orYYYY-MM-DDTHH:mm:ssZto avoid malformed front matter.- date: ${moment().format('YYYY-MM-DDTh:mm:ssZ')} + date: ${moment().toISOString()}
35-41: Harden tag parsing (trim, drop empties, escape quotes).Avoid empty entries on trailing commas and escape single quotes inside tags to keep YAML valid.
- const tagArray = answers.tags.split(','); - - tagArray.forEach((tag: string, index: number) => { - tagArray[index] = tag.trim(); - }); - const tags = `'${tagArray.join("','")}'`; + const tagArray = (answers.tags || '') + .split(',') + .map(t => t.trim()) + .filter(Boolean); + const tags = tagArray + .map(t => `'${t.replace(/'/g, "''")}'`) + .join(', ');- tags: [${answers.tags ? tags : ''}] + tags: [${tagArray.length ? tags : ''}]Also applies to: 47-48
tests/adopters/index.test.ts (1)
6-6: Mock path must match the import specifier.You import without an extension but mock with
.ts, which can prevent Jest from mocking correctly.-jest.mock('../../scripts/helpers/readAndWriteJson.ts'); +jest.mock('../../scripts/helpers/readAndWriteJson', () => ({ + writeJSON: jest.fn(), +}));scripts/tools/tools-object.ts (1)
109-116: Use robust ref extraction from URL; avoid brittle split('=')[1].
tool.urlparsing via split is fragile. Use URLSearchParams to readref, and fallback defensively.- const referenceId = tool.url.split('=')[1]; - const downloadUrl = `https://raw.githubusercontent.com/${tool.repository.full_name}/${referenceId}/${tool.path}`; + const u = new URL(tool.url); + const referenceId = u.searchParams.get('ref') || tool.repository.default_branch || 'main'; + const downloadUrl = `https://raw.githubusercontent.com/${tool.repository.full_name}/${referenceId}/${tool.path}`;scripts/build-newsroom-videos.ts (1)
37-46: Fix YouTube Search API order param (must be lowercase 'date'); consider removing eventType filterThe API expects order='date' (lowercase). 'Date' can break or be ignored. Also, eventType='completed' restricts results to live streams; drop it if you want all videos.
new URLSearchParams({ key: process.env.YOUTUBE_TOKEN, part: 'snippet', channelId: 'UCIz9zGwDLbrYQcDKVXdOstQ', - eventType: 'completed', type: 'video', - order: 'Date', + order: 'date', maxResults: '5' })}
🧹 Nitpick comments (90)
types/pages/community/Community.ts (1)
39-53: Minor typing cleanups possible in Tsc interface
- Consider replacing
repos: anywith a stricter shape (evenunknown+ a narrow type where used) to avoid unsafe access.- Social/contact duplication: you have both
github/githubUrl?/twitterUrl?/linkedinUrl?. Unify to either handles or URLs, or introduce aSocialtype to avoid divergence.- Fields like
name,github, andisTscMembercould bereadonlyif not mutated at runtime.No blockers—current change (semicolon on
githubID) improves consistency.types/typography/Paragraph.ts (2)
4-5: Consider adding a trailing comma after the last enum member.Reduces diff noise in future edits and aligns with common TS style.
- sm = 'body-sm' + sm = 'body-sm',
1-5: Optional: prefer a union type to avoid runtime enum emission.If this is used purely for typing/CSS class strings, a union type (and optional const array) avoids bundling a runtime enum object.
-export enum ParagraphTypeStyle { - lg = 'body-lg', - md = 'body-md', - sm = 'body-sm' -} +export type ParagraphTypeStyle = 'body-lg' | 'body-md' | 'body-sm'; +export const ParagraphTypeStyles = ['body-lg', 'body-md', 'body-sm'] as const;scripts/tools/categorylist.ts (3)
52-53: Make spacing explicit and unify quotes.Leading space in the second literal is clearer than a trailing space in the first; also keep quotes consistent with the file’s single-quote style.
- "Writing YAML by hand is no fun, and maybe you don't want a GUI, " + - 'so use a Domain Specific Language to write AsyncAPI in your language of choice.' + 'Writing YAML by hand is no fun, and maybe you don\'t want a GUI,' + + ' so use a Domain Specific Language to write AsyncAPI in your language of choice.'
69-70: Same here: explicit inter-string space and quote consistency.- 'The tools below take specification documents as input, then publish fake messages to broker ' + - 'destinations for simulation purposes. They may also check that publisher messages are compliant with schemas.' + 'The tools below take specification documents as input, then publish fake messages to broker' + + ' destinations for simulation purposes. They may also check that publisher messages are compliant with schemas.'
101-102: Ditto: avoid trailing-space-in-string; keep single quotes.- 'The following is a list of templates compatible with AsyncAPI Generator. ' + - 'You can use them to generate apps, clients or documentation from your AsyncAPI documents.' + 'The following is a list of templates compatible with AsyncAPI Generator.' + + ' You can use them to generate apps, clients or documentation from your AsyncAPI documents.'scripts/markdown/check-edit-links.ts (5)
41-49: Type fix LGTM; also make ignoreFiles check OS-agnostic.ReturnType is the right cross-runtime type. One nit: endsWith against filePath breaks on Windows separators. Normalize before checking.
- if (!editLink || ignoreFiles.some((ignorePath) => filePath.endsWith(ignorePath))) return null; + const normalizedFilePath = filePath.split(path.sep).join('/'); + if (!editLink || ignoreFiles.some((ignorePath) => normalizedFilePath.endsWith(ignorePath))) return null;Optional: if your runtime supports it, prefer AbortSignal.timeout(TIMEOUT_MS) and drop the manual timer.
61-61: Don’treturn await Promise.reject(...); just throw with context.Using return await Promise.reject in async is redundant and obscures the stack. Throw directly and include the original message.
- return await Promise.reject(new Error(`Error checking ${editLink}: ${error}`)); + throw new Error(`Error checking ${editLink}: ${(error as Error)?.message ?? String(error)}`);(Optional: if targeting ES2022 libs, use
new Error(msg, { cause: error }).)
74-76: Clarify concurrency note in docs.Saying “processes them concurrently” plus “after processing each batch and pausing” can read as sequential. Consider clarifying that the pause is per-batch but batches still run in parallel.
201-203: Fix minor grammar in comment.- * and generates a list of markdown file paths with their associated edit links. It checks the validity of each URL - * and logs - * any links that return a 404 status. If an error occurs during processing, an error is thrown summarizing the failure. + * and generates a list of markdown file paths with their associated edit links. It checks the validity of each URL + * and logs any links that return a 404 status. If an error occurs during processing, an error is thrown summarizing the failure.
221-223: Log the 404 list in one call to avoid interleaving and extra newlines.- invalidUrls.forEach((url) => { - logger.info(`- ${url.editLink} generated from ${url.filePath}\n`); - }); + const list = invalidUrls + .map((u) => `- ${u.editLink} generated from ${u.filePath}`) + .join('\n'); + logger.info(list);types/components/tools/ToolDataType.ts (1)
1-1: Type-only import LGTM; tighten elementRef genericNice switch to a type-only import. Consider narrowing away from any to improve safety (e.g., RefObject or RefObject) based on usage.
- elementRef?: RefObject<any>; + elementRef?: RefObject<HTMLElement>;Also applies to: 46-46
.eslintignore (1)
1-14: Expand ignores for common Next/monorepo artifacts; confirm intent to ignore config filesAdditions that often reduce noise:
- .vercel/
- .turbo/
- .eslintcache
- storybook-static/
- playwright-report/
- .cache/
Also, you ignore *.config.{js,mjs,cts} but not {cjs,ts,mts}. If the intent is “ignore all config files,” broaden the pattern; if not, leave as-is.
node_modules/ .next/ out/ public/ *.config.js *.config.mjs *.config.cts +.config.cjs +*.config.ts +*.config.mts .storybook/ coverage/ netlify/ cypress/ dist/ build/ next-env.d.ts +.vercel/ +.turbo/ +.eslintcache +storybook-static/ +playwright-report/ +.cache/mdx-components.tsx (1)
10-16: Doc add LGTM; consider Partial type and safe default for componentsMDX providers may pass partial or undefined overrides. Accept Partial and default to {} to avoid spreading undefined.
-export function useMDXComponents(components: MDXComponents): MDXComponents { +export function useMDXComponents(components: Partial<MDXComponents> = {}): MDXComponents { return { // Allows customizing built-in components, e.g. to add styling. ...mdxComponents, ...components }; }types/components/IconType.ts (1)
1-1: Type-only import LGTM; avoid any in propsIf these are SVG icons, prefer SVGProps. If mixed, consider a generic props type or ComponentType.
-import type { ReactElement } from 'react'; - -export type IconType = (props: any) => ReactElement; +import type { ReactElement, SVGProps } from 'react'; + +export type IconType = (props: SVGProps<SVGSVGElement>) => ReactElement;Also applies to: 3-3
scripts/tools/extract-tools-github.ts (1)
71-73: Align with runner error handling; rethrow with context (CustomError)Per repo conventions, wrap and rethrow with context instead of bare throw.
- } catch (err) { - throw err; - } + } catch (err) { + // Consider importing your CustomError type if available in this repo + // throw new CustomError('TOOLS_GITHUB_FETCH_FAILED', 'Failed to fetch tools from GitHub', { page, perPage: maxPerPage, cause: err }); + const e = err as Error; + throw new Error(`Failed to fetch tools from GitHub (page=${page}) - ${e.message}`); + }If you want, I can patch this to use your CustomError type—confirm its path and shape.
.eslintrc (1)
65-66: Include .spec.ts in devDependencies globs.Spec TS files aren’t covered; add it for parity with .spec.js.
Apply:
- { "devDependencies": ["**/*.test.js", "**/*.test.ts", "**/*.spec.js", "**/setupTests.js", "tests/**/*"] } + { "devDependencies": ["**/*.test.js", "**/*.test.ts", "**/*.spec.js", "**/*.spec.ts", "**/setupTests.js", "tests/**/*"] }tests/dashboard/build-dashboard.test.ts (1)
39-42: Prefer consistent fs-extra API usage.Instead of importing promises from fs-extra, import fs-extra once and use its promise-returning methods for consistency.
-import { promises as fs, rmSync } from 'fs-extra'; +import fs, { rmSync } from 'fs-extra'; ... - await fs.mkdir(tempDir, { recursive: true }); + await fs.mkdir(tempDir, { recursive: true });Also applies to: 49-54
scripts/build-post-list.ts (2)
190-196: Deterministic traversal: sort directory entries.readdir order isn’t guaranteed; sorting stabilizes output and tests.
- const files = await readdir(directory); + const files = (await readdir(directory)).sort();
265-271: Release notes detection may be order-dependent.If a spec page is processed before its corresponding release-notes file, releaseNoteLink won’t be set. Consider first collecting release notes (pre-scan) or linking in a post-pass.
scripts/compose.ts (2)
113-121: MDX/React attributes: replace deprecated/invalid HTML.
<center>is deprecated andframeborderis not a valid React prop. Center with a div and useframeBorder.- <center> - <iframe + <div style={{ textAlign: 'center' }}> + <iframe src="https://anchor.fm/asyncapi/embed/episodes/April-2021-at-AsyncAPI-Initiative-e111lo9" height="102px" width="400px" - frameborder="0" + frameBorder="0" scrolling="no" /> - </center> + </div>
14-20: Type shape is fine; consider future-proofing tags/type.Optionally accept
tags: string | string[]and narrowtypeto a union if values are constrained (e.g., 'blog' | 'news'). Not blocking.types/errors/CustomError.ts (1)
22-28: Improve stack capture for non-Error inputs.Handle strings and JSON-serializable objects to retain context instead of emitting “Unknown error”.
function captureFormattedStack(error: Error | unknown): string { - if (error instanceof Error) { - return error.stack || error.message; - } - - return new Error('Unknown error').stack || ''; + if (error instanceof Error) return error.stack || error.message; + if (typeof error === 'string') return error; + try { + return JSON.stringify(error, null, 2); + } catch { + return String(error); + } }tests/adopters/index.test.ts (1)
8-16: Stabilize mocks across tests.Provide a default resolved value and reset after each test to avoid bleed-through.
describe('buildAdoptersList', () => { + beforeEach(() => { + (writeJSON as jest.Mock).mockResolvedValue(undefined); + }); + + afterEach(() => { + jest.resetAllMocks(); + });- // Reset mock for other tests - (writeJSON as jest.Mock).mockResolvedValue(undefined); + // no-op: afterEach resets all mocksAlso applies to: 25-27
scripts/casestudies/index.ts (1)
22-26: Make output deterministic and safer (filter + sort).Directory order isn’t guaranteed; also avoid attempting to parse non-YAML files.
Apply:
- const files = await readdir(dirWithCaseStudy); + const files = (await readdir(dirWithCaseStudy)) + .filter((f) => /\.ya?ml$/i.test(f)) + .sort();scripts/helpers/readAndWriteJson.ts (1)
21-41: Prefer throwing in async functions over returning Promise.reject/resolve.Simplifies control flow and stack traces.
Apply:
try { readContent = await readFile(readPath, 'utf-8'); } catch (err) { - return Promise.reject(err); + throw err; } @@ try { jsonContent = convertToJson(readContent); } catch (err) { - return Promise.reject(err); + throw err; } @@ try { await writeFile(writePath, JSON.stringify(jsonContent)); } catch (err) { - return Promise.reject(err); + throw err; } - return Promise.resolve(); + return;tests/build-tools.test.ts (1)
51-54: Restore env after tests.Avoid leaking GITHUB_TOKEN into other tests.
Apply:
afterAll(() => { fs.removeSync(testDir); consoleErrorMock.mockRestore(); + delete process.env.GITHUB_TOKEN; });scripts/finance/index.ts (1)
50-57: Parallelize independent writes.Minor speedup and symmetry.
Apply:
- const expensesJsonPath = resolve(jsonDirectory, 'Expenses.json'); - await writeJSON(expensesPath, expensesJsonPath); - - const expensesLinkJsonPath = resolve(jsonDirectory, 'ExpensesLink.json'); - await writeJSON(expensesLinkPath, expensesLinkJsonPath); + const expensesJsonPath = resolve(jsonDirectory, 'Expenses.json'); + const expensesLinkJsonPath = resolve(jsonDirectory, 'ExpensesLink.json'); + await Promise.all([ + writeJSON(expensesPath, expensesJsonPath), + writeJSON(expensesLinkPath, expensesLinkJsonPath) + ]);tests/casestudies/index.test.ts (1)
48-53: Use rejects matcher to simplify error assertions.Cleaner and ensures the assertion runs.
Apply:
- it('should throw an error with incorrect parameters', async () => { - try { - await buildCaseStudiesList('invalid-dir', tempOutputFile); - } catch (error) { - expect(error).toBeInstanceOf(CustomError); - } - }); + it('should throw an error with incorrect parameters', async () => { + await expect(buildCaseStudiesList('invalid-dir', tempOutputFile)) + .rejects.toBeInstanceOf(CustomError); + }); - it('should throw an error when the output file path is invalid', async () => { - try { - // Call the function with an invalid output file path - await buildCaseStudiesList(tempConfigDir, '/invalid-path/case-studies.json'); - } catch (error) { - expect(error).toBeInstanceOf(CustomError); - } - }); + it('should throw an error when the output file path is invalid', async () => { + await expect(buildCaseStudiesList(tempConfigDir, '/invalid-path/case-studies.json')) + .rejects.toBeInstanceOf(CustomError); + }); - it('should throw an error when YAML content is invalid', async () => { + it('should throw an error when YAML content is invalid', async () => { @@ - try { - await buildCaseStudiesList(tempConfigDir, tempOutputFile); - } catch (error) { - expect(error).toBeInstanceOf(CustomError); - } + await expect(buildCaseStudiesList(tempConfigDir, tempOutputFile)) + .rejects.toBeInstanceOf(CustomError); });Also applies to: 56-63, 73-77
npm/runners/build-post-list-runner.ts (3)
11-15: Type tuple precisely for postDirectories.Improves safety and IDE help.
Apply:
-interface BuildPostListOptions { - postDirectories?: string[][]; +interface BuildPostListOptions { + postDirectories?: Array<[string, string]>; basePath?: string; outputPath?: string; }
28-31: Export the runner for tests and reuse.Enables invoking with custom options in unit/integration tests.
Apply:
-async function runBuildPostList(options: BuildPostListOptions = {}): Promise<void> { +export async function runBuildPostList(options: BuildPostListOptions = {}): Promise<void> {
41-45: Use resolved outputPath in error detail (not options.outputPath).Avoids “undefined” in error context when using defaults.
Apply:
- detail: `Build post list failed with output path: ${options.outputPath}` + detail: `Build post list failed with output path: ${resolve(currentDirPath, '../../config', 'posts.json')}`Alternatively capture
outputPathabove and interpolate that variable.scripts/tools/combine-tools.ts (3)
150-155: Clarify log payload shape.Key name “tool” holds a title string; make it explicit to avoid confusion.
Apply:
- logger.error('Tool validation failed', { - tool: tool.title, + logger.error('Tool validation failed', { + toolTitle: tool.title, source: 'manual-tools.json', errors: validate.errors, note: 'Script continues execution, error logged for investigation' });
147-148: Drop unnecessary await on synchronous AJV validation.Ajv validate returns boolean.
Apply:
- const isValid = await validate(tool); + const isValid = validate(tool);
220-221: Align error model with CustomError.Other scripts now emit CustomError; consider doing the same for consistency.
Apply:
- } catch (err) { - throw new Error(`Error combining tools: ${err}`); + } catch (err) { + throw CustomError.fromError(err, { + category: 'script', + operation: 'combineTools', + detail: 'Failed to combine automated and manual tools' + }); }Add import at top:
import { CustomError } from '@/types/errors/CustomError';tests/build-rss.test.ts (3)
129-130: Strengthen assertion by checking error metadata (malformed posts.json).You can assert the CustomError shape to ensure we keep category/operation contracts stable.
- await expect(rssFeed(type, title, desc, outputPath)).rejects.toThrow(CustomError); + await expect(rssFeed(type, title, desc, outputPath)).rejects.toMatchObject({ + name: 'CustomError', + category: 'script', + operation: 'rssFeed', + });
149-150: Also assert error metadata for “missing required fields”.Validates the validation-path contract.
- await expect(rssFeed(type, title, desc, outputPath)).rejects.toThrow(CustomError); + await expect(rssFeed(type, title, desc, outputPath)).rejects.toMatchObject({ + name: 'CustomError', + category: 'validation', + operation: 'rssFeed', + });
166-167: Also assert error metadata for “missing dates”.Keeps the validation-path consistent across scenarios.
- await expect(rssFeed(type, title, desc, outputPath)).rejects.toThrow(CustomError); + await expect(rssFeed(type, title, desc, outputPath)).rejects.toMatchObject({ + name: 'CustomError', + category: 'validation', + operation: 'rssFeed', + });Please confirm the “write operation fails” test (Lines 112-118) still passes now that non-CustomError errors are wrapped. If it starts failing, switch it to expect CustomError and check the detail includes the output path.
scripts/build-rss.ts (4)
51-57: Include identifiers in detail for easier triage.You already list offending posts in the message. Mirroring a short list in detail helps logs and tests without parsing message text.
- detail: `Missing date in ${missingDatePosts.length} posts for RSS feed: ${rssTitle}` + detail: `Missing date in ${missingDatePosts.length} posts for RSS feed: ${rssTitle} — ${missingDatePosts + .slice(0, 10) + .map((p) => p.title || p.slug) + .join(', ')}`
85-90: Likewise, surface which posts are invalid in detail.- detail: `Missing required fields in ${invalidPosts.length} posts: ${invalidPosts.map((p) => p.title || p.slug).join(', ')}` + detail: `Missing required fields in ${invalidPosts.length} posts: ${invalidPosts + .slice(0, 20) + .map((p) => p.title || p.slug) + .join(', ')}`
91-104: Hoist mime map to module scope to avoid reallocation.Micro perf/readability.
- const mimeTypes: { - [key: string]: string; - } = { - '.jpeg': 'image/jpeg', - '.jpg': 'image/jpeg', - '.png': 'image/png', - '.svg': 'image/svg+xml', - '.webp': 'image/webp', - '.gif': 'image/gif', - '.bmp': 'image/bmp', - '.tiff': 'image/tiff', - '.ico': 'image/x-icon' - }; + // uses top-level MIME_TYPESAdd at top-level (outside the function):
const MIME_TYPES: Record<string, string> = { '.jpeg': 'image/jpeg', '.jpg': 'image/jpeg', '.png': 'image/png', '.svg': 'image/svg+xml', '.webp': 'image/webp', '.gif': 'image/gif', '.bmp': 'image/bmp', '.tiff': 'image/tiff', '.ico': 'image/x-icon', };And inside the loop:
- const mimeType = mimeTypes[fileExtension] || 'image/jpeg'; + const mimeType = MIME_TYPES[fileExtension] || 'image/jpeg';
139-150: Preserve underlying errno/code for wrapped errors.Ensure ENOENT/EACCES (and others) remain accessible via message or a code/cause property so existing tests and diagnostics don’t lose fidelity.
If
CustomError.fromErrorsupports it, propagate the original error ascauseand copycode:- throw CustomError.fromError(err, { + throw CustomError.fromError(err, { category: 'script', operation: 'rssFeed', - detail: `Failed to generate RSS feed - type: ${type}, title: ${rssTitle}, output: ${outputPath}` + detail: `Failed to generate RSS feed - type: ${type}, title: ${rssTitle}, output: ${outputPath}`, + // optional, if supported by your CustomError + code: (err as NodeJS.ErrnoException)?.code, + cause: err as Error, });scripts/build-pages.ts (3)
38-45: Minor: faster membership check and slightly clearer replacer.-const capitalizeTags = ['table', 'tr', 'td', 'th', 'thead', 'tbody']; +const capitalizeTags = new Set(['table', 'tr', 'td', 'th', 'thead', 'tbody']); ... - return content.replace(/<\/?(\w+)/g, (match: string, letter: string): string => { - if (capitalizeTags.includes(letter.toLowerCase())) { + return content.replace(/<\/?(\w+)/g, (match: string, tag: string): string => { + if (capitalizeTags.has(tag.toLowerCase())) { - return `<${match[1] === '/' ? '/' : ''}${letter[0].toUpperCase()}${letter.slice(1)}`; + return `<${match[1] === '/' ? '/' : ''}${tag[0].toUpperCase()}${tag.slice(1)}`; } return match; });
74-75: Remove no-op replacement.This line doesn’t change content and adds cost.
- content = content.replace(/{/g, '{');
84-86: Make .md → .mdx rename case-insensitive.Prevents misses on “.MD”.
- if (path.extname(targetPath) === '.md') { + if (path.extname(targetPath).toLowerCase() === '.md') { fs.renameSync(targetPath, `${targetPath.slice(0, -3)}.mdx`); }npm/runners/build-adopters-list-runner.ts (2)
22-24: Use structured error payload with logger for consistency.Prefer passing
{ error: customError }to keep log shape uniform with other modules.- logger.error('Build adopters list runner failed', customError); + logger.error('Build adopters list runner failed', { error: customError });
28-35: Export the runner for testability (aligns with other runners).Other runners export their
run*function; do the same here for unit tests.Add below the IIFE:
export { runBuildAdoptersList };npm/runners/build-pages-runner.ts (1)
41-44: Log structured error payload.Standardize on
{ error }as the second argument.- logger.error('Build pages runner failed', customError); + logger.error('Build pages runner failed', { error: customError });scripts/markdown/check-markdown.ts (1)
156-159: Harden skip logic to match directory boundaries.
includes(specPath)may match unintended paths. Consider comparing path segments.- const specPath = path.join('reference', 'specification'); - if (path.normalize(relativeFilePath).includes(specPath)) { + const specPath = path.normalize(path.join('reference', 'specification') + path.sep); + if (path.normalize(relativeFilePath).startsWith(specPath)) { return; }npm/runners/build-meetings-runner.ts (1)
25-41: Use resolved output path in error detail and structured logging.Detail currently prints
undefinedwhen no option is provided; also standardize logger payload.-async function runBuildMeetings(options: BuildMeetingsOptions = {}): Promise<void> { - try { - const outputPath = options.outputPath || resolve(currentDirPath, '../../config', 'meetings.json'); - - await buildMeetings(outputPath); +async function runBuildMeetings(options: BuildMeetingsOptions = {}): Promise<void> { + const resolvedOutputPath = options.outputPath || resolve(currentDirPath, '../../config', 'meetings.json'); + try { + await buildMeetings(resolvedOutputPath); } catch (error) { const customError = CustomError.fromError(error, { category: 'script', operation: 'runBuildMeetings', - detail: `Build meetings failed with output path: ${options.outputPath}` + detail: `Build meetings failed with output path: ${resolvedOutputPath}` }); - logger.error('Build meetings runner failed', customError); + logger.error('Build meetings runner failed', { error: customError }); throw customError; } }npm/runners/case-studies-runner.ts (2)
44-53: Structured logging and resolved paths in error detail.- const customError = CustomError.fromError(error, { + const customError = CustomError.fromError(error, { category: 'script', operation: 'runCaseStudies', - detail: `Case studies build failed - directory: ${options.caseStudyDirectory}, output: ${options.outputPath}` + detail: `Case studies build failed - directory: ${caseStudyDirectory}, output: ${writeFilePath}` }); - logger.error('Case studies runner failed', customError); + logger.error('Case studies runner failed', { error: customError });
56-65: Skip auto-run in tests and export the runner (align with other runners).-// Self-executing async function to handle top-level await -(async () => { - try { - await runCaseStudies(); - } catch (error) { - // Ensure we exit with error code - process.exit(1); - } -})(); +// Run only in non-test environments +if (process.env.NODE_ENV === 'test') { + logger.info('Skipping case studies build in test environment'); +} else { + (async () => { + try { + await runCaseStudies(); + } catch { + process.exit(1); + } + })(); +} + +// Export for testing +export { runCaseStudies };npm/runners/compose-blog-runner.ts (2)
86-93: Validate/normalize tags to expected type.If
ComposePromptType['tags']isstring[], split and trim the input.- return { - title, - excerpt, - tags, - type, - canonical - }; + return { + title, + excerpt, + tags: tags.split(',').map((t) => t.trim()).filter(Boolean), + type, + canonical + };
50-52: Structured error payload with logger.- logger.error('Compose blog runner failed', customError); + logger.error('Compose blog runner failed', { error: customError });npm/runners/build-newsroom-videos-runner.ts (2)
25-35: Use the resolved outputPath in error context (avoid “undefined”).
detaillogsoptions.outputPath, which is undefined when using the default. Capture the resolved path outside try and reference it in the catch.-async function runBuildNewsroomVideos(options: BuildNewsroomVideosOptions = {}): Promise<void> { - try { - const outputPath = options.outputPath || resolve(currentDirPath, '../../config', 'newsroom_videos.json'); - - await buildNewsroomVideos(outputPath); +async function runBuildNewsroomVideos(options: BuildNewsroomVideosOptions = {}): Promise<void> { + const resolvedOutputPath = + options.outputPath || resolve(currentDirPath, '../../config', 'newsroom_videos.json'); + try { + await buildNewsroomVideos(resolvedOutputPath); } catch (error) { const customError = CustomError.fromError(error, { category: 'script', operation: 'runBuildNewsroomVideos', - detail: `Build newsroom videos failed with output path: ${options.outputPath}` + detail: `Build newsroom videos failed with output path: ${resolvedOutputPath}` });
47-55: Prefer setting exitCode to allow logs to flush.Optional:
process.exit(1)can truncate logs in some environments. Useprocess.exitCode = 1and let the event loop drain.- } catch (error) { - // Ensure we exit with error code - process.exit(1); - } + } catch (error) { + // Ensure we exit with error code while letting logs flush + process.exitCode = 1; + }npm/runners/build-dashboard-runner.ts (2)
25-42: Preserve and log the resolved output path (not the possibly-undefined option).Move resolution outside try so the catch can reliably include the actual path. Also ensures consistent context whether default or provided.
-async function runBuildDashboard(options: BuildDashboardOptions = {}): Promise<void> { - try { - const outputPath = options.outputPath || resolve(currentDirPath, '../../dashboard.json'); - - await buildDashboard(outputPath); +async function runBuildDashboard(options: BuildDashboardOptions = {}): Promise<void> { + const resolvedOutputPath = + options.outputPath || resolve(currentDirPath, '../../dashboard.json'); + try { + await buildDashboard(resolvedOutputPath); } catch (error) { // Create or enhance the error with full context const customError = error instanceof CustomError ? error.updateContext({ operation: 'runBuildDashboard', - detail: `Runner failed with output path: ${options.outputPath}` + detail: `Runner failed with output path: ${resolvedOutputPath}` }) : CustomError.fromError(error, { category: 'script', operation: 'runBuildDashboard', - detail: `Build dashboard runner failed with output path: ${options.outputPath}` + detail: `Build dashboard runner failed with output path: ${resolvedOutputPath}` });
51-64: Same exit behavior nit as other runner.- } catch (error) { - // Ensure we exit with error code - process.exit(1); - } + } catch (error) { + // Ensure we exit with error code while letting logs flush + process.exitCode = 1; + }scripts/tools/tools-object.ts (5)
131-143: Simplify category assignment; avoid unnecessary async/Promise.all.The inner map is sync; remove async wrapper and Promise.all. Also guard categories just in case schema evolves.
- // Tool Object is appended to each category array according to Fuse search for categories inside - // Tool Object - await Promise.all( - (jsonToolFileContent as unknown as AsyncAPITool).filters.categories.map(async (category: string) => { - const categorySearch = fuse.search(category); - const targetCategory = categorySearch.length ? categorySearch[0].item.name : 'Others'; - const { toolsList } = finalToolsObject[targetCategory]; - - if (!toolsList.includes(toolObject)) { - toolsList.push(toolObject); - } - }) - ); + // Append Tool Object to each category according to Fuse search + for (const category of toolData.filters?.categories ?? []) { + const categorySearch = fuse.search(category); + const targetCategory = categorySearch.length ? categorySearch[0].item.name : 'Others'; + const { toolsList } = finalToolsObject[targetCategory]; + if (!toolsList.includes(toolObject)) { + toolsList.push(toolObject); + } + }
145-155: Validation failure handling changed to warn-and-continue; confirm desired behavior.Past guidance favored throwing on validation failures to surface data issues early. If best-effort is intended, consider a
strictflag (default true) to fail CI while allowing local leniency.Proposed API tweak (outside this hunk):
export async function convertTools(data: ToolsData, options: { strict?: boolean } = { strict: true }) { /* ... */ }Then:
if (!isValid) { const ctx = { /* existing structured log */ }; logger.warn('Invalid .asyncapi-tool file detected', ctx); if (options.strict) throw new Error(`Invalid .asyncapi-tool file: ${tool.name}`); continue; }
112-116: Set a request timeout to avoid hanging on slow GitHub responses.Unbounded axios calls can stall the whole build. Add a conservative timeout and maybe limited retries.
- const { data: toolFileContent } = await axios.get(downloadUrl); + const { data: toolFileContent } = await axios.get(downloadUrl, { timeout: 15000 });
31-46: Docstring matches current async contract; keep as-is for compatibility.If we later make
createToolObjectsync, align JSDoc and call sites in one PR.
13-16: Optional: capture all schema errors for richer logs.Initialize Ajv with
allErrors: trueto report all validation issues.Example (outside this hunk):
const ajv = new Ajv({ allErrors: true });npm/runners/build-tools-runner.ts (1)
64-77: Top-level catch should log before exit (align with runners pattern).Per retrieved learnings, the orchestrator logs a task-specific error. Keep the log here, then exit.
(async () => { try { await runBuildTools(); } catch (error) { - // Ensure we exit with error code + logger.error('Error building tools:', error); process.exit(1); } })();I used the stored learnings about runner error handling in this suggestion.
tests/build-meetings.test.ts (2)
6-6: Good move to assert CustomError; consider asserting category/operation too.Current checks only validate the error type. Increase signal by matching key fields (e.g., category: 'script', operation: 'buildMeetings') if available on CustomError.
Example:
await expect(buildMeetings(outputFilePath)).rejects.toMatchObject({ category: 'script', operation: 'buildMeetings' });Also applies to: 71-72, 79-80, 91-92, 101-102, 118-119, 121-136, 145-146, 159-160
89-92: Make file-write error deterministic and cross‑platform.Using “/root/…” assumes Unix perms. Force an EISDIR error by trying to write to a directory path; works on all OSes.
- const invalidPath = '/root/invalid_dir/meetings.json'; - await expect(buildMeetings(invalidPath)).rejects.toThrow(CustomError); + const invalidPath = path.join(testDir, 'meetings.json'); + mkdirSync(invalidPath, { recursive: true }); // create a directory named 'meetings.json' + await expect(buildMeetings(invalidPath)).rejects.toThrow(CustomError);scripts/build-tools.ts (2)
33-36: Prefer fs-extra helpers for robustness (ensures dirs, simpler JSON IO).Use outputFile/readJSON to avoid manual JSON.parse and directory existence issues.
- await fs.writeFile(automatedToolsPath, JSON.stringify(automatedTools, null, ' ')); + await fs.outputFile(automatedToolsPath, JSON.stringify(automatedTools, null, 2)); ... - const manualTools = JSON.parse(await fs.readFile(manualToolsPath, 'utf-8')); + const manualTools = await fs.readJSON(manualToolsPath);
39-43: Include tagsPath in error detail; keeps diagnostics complete.- detail: `Failed to build tools - automated: ${automatedToolsPath}, manual: ${manualToolsPath}, output: ${toolsPath}` + detail: `Failed to build tools - automated: ${automatedToolsPath}, manual: ${manualToolsPath}, tools: ${toolsPath}, tags: ${tagsPath}`tests/markdown/check-markdown.test.ts (4)
103-109: Tighten the assertion to verify the exact folderPath passed to logger.error.Asserting the concrete path prevents false positives from prior calls.
- expect(mockLoggerError).toHaveBeenCalledWith( + expect(mockLoggerError).toHaveBeenCalledWith( expect.stringContaining('Error in directory'), expect.objectContaining({ - error: expect.any(Error), - folderPath: expect.any(String) + error: expect.any(Error), + folderPath: invalidFolderPath }) );
151-157: Include filePath in the structured payload for per-file failures (then assert it here).Having filePath alongside folderPath makes triage faster. Apply if you add filePath in scripts/markdown/check-markdown.ts.
- expect(logger.error).toHaveBeenCalledWith( + expect(logger.error).toHaveBeenCalledWith( expect.stringContaining('Error in directory'), expect.objectContaining({ error: expect.any(Error), - folderPath: expect.any(String) + folderPath: expect.any(String), + filePath: expect.any(String) }) );
168-173: Prevent cross-test pollution by clearing mocks after each test.Some specs rely on call history; clearing avoids accidental passes.
Add to your afterEach block:
afterEach(async () => { mockConsoleError.mockRestore(); mockProcessExit.mockRestore(); await fs.rm(tempDir, { recursive: true, force: true }); + jest.clearAllMocks(); });
178-192: Assert that non-Error rejections preserve the original message.This ensures CustomError.fromError wraps string errors without losing context.
- expect(logger.error).toHaveBeenCalledWith( + expect(logger.error).toHaveBeenCalledWith( 'Failed to validate markdown files', expect.objectContaining({ - error: expect.any(Error) + error: expect.objectContaining({ message: 'String error' }) }) );npm/runners/build-finance-info-list-runner.ts (2)
33-41: Hoist targetYear to include it in error context.Let catch blocks report the resolved year (auto or provided) for better diagnostics.
-async function runBuildFinanceInfoList(options: BuildFinanceInfoOptions = {}): Promise<void> { - const config = { ...DEFAULT_OPTIONS, ...options }; - - const financeDir = resolve(config.currentDir, config.configDir, config.financeDir); - - try { - // If year is not provided, find the latest year - let targetYear = config.year; +async function runBuildFinanceInfoList(options: BuildFinanceInfoOptions = {}): Promise<void> { + const config = { ...DEFAULT_OPTIONS, ...options }; + const financeDir = resolve(config.currentDir, config.configDir, config.financeDir); + let targetYear = config.year; + try { + // If year is not provided, find the latest year @@ - const customError = CustomError.fromError(error, { + const customError = CustomError.fromError(error, { category: 'script', operation: 'runBuildFinanceInfoList', - detail: `Build finance info failed for year: ${config.year}, financeDir: ${financeDir}` + detail: `Build finance info failed for year: ${targetYear ?? 'auto'}, financeDir: ${financeDir}` });Also applies to: 71-75
77-79: Unify error message wording with other runners.Keeps logs grep-friendly and consistent.
- logger.error('Build finance info list runner failed', customError); + logger.error('Error building finance info list', customError);tests/tools/combine-tools.test.ts (1)
34-54: Mimic Ajv behavior: reset validate.errors to null on success.Prevents stale errors leaking across validations when the same validate fn is reused.
- function validate(data: any): boolean { + function validate(data: any): boolean { const isValid = data.title !== 'Invalid Tool'; - - if (!isValid) { - (validate as any).errors = [ - { - instancePath: '/title', - schemaPath: '#/properties/title', - keyword: 'invalid', - message: 'Invalid tool title' - } - ]; - } - - return isValid; + (validate as any).errors = isValid + ? null + : [ + { + instancePath: '/title', + schemaPath: '#/properties/title', + keyword: 'invalid', + message: 'Invalid tool title' + } + ]; + return isValid; }scripts/helpers/check-locales.ts (3)
60-68: Add nodeError to JSON parse/read error logs for parity with directory errorsInclude the Node error code when available to improve triage consistency.
- logger.error('Failed to parse JSON locale file', { + logger.error('Failed to parse JSON locale file', { error: error instanceof Error ? error : new Error(String(error)), operation: 'parseJSON', filePath, fileName: file, directory: dir, fileExtension: path.extname(file), - errorType: error instanceof SyntaxError ? 'JSON_SYNTAX_ERROR' : 'FILE_READ_ERROR' + errorType: error instanceof SyntaxError ? 'JSON_SYNTAX_ERROR' : 'FILE_READ_ERROR', + nodeError: error instanceof Error && 'code' in error ? (error as any).code : undefined });
157-167: Guard against excessive log volume for large missing key setsWhen many keys are missing, logging the entire list can be noisy. Consider sampling the first N keys and including counts.
- logger.error(`Translation keys missing in language '${lang}'`, { + logger.error(`Translation keys missing in language '${lang}'`, { operation: 'validateTranslationKeys', fileName: file, language: lang, - missingKeys: missing, + missingKeys: missing.length > 50 ? missing.slice(0, 50) : missing, + missingKeysSampled: missing.length > 50, missingKeyCount: missing.length, totalKeysInFile: allKeysAcrossLanguages.length, errorType: 'MISSING_TRANSLATION_KEYS', affectedLanguages: langsWithMissingKeys.map(([l]) => l), languagesWithFile: langsWithFile });
130-131: Avoid recomputing per-file issue stats in validationSummaryTrack files with issues during the main loop and reuse, reducing duplicate work on large locale sets.
let hasErrors = false; + const filesWithIssues = new Set<string>(); @@ if (langsWithMissingKeys.length > 0) { @@ - hasErrors = true; + hasErrors = true; + filesWithIssues.add(file); } @@ - const validationSummary = { - totalFiles: allFiles.length, - totalLanguages: languages.length, - filesWithIssues: allFiles.filter((file) => { - const langsWithFile = languages.filter((lang) => languageFiles[lang][file]); - - if (langsWithFile.length <= 1) return false; - const allKeysAcrossLanguages = uniq(flatten(langsWithFile.map((lang) => fileKeys[lang][file]))); - const missingKeysByLang = fromPairs( - langsWithFile.map((lang) => { - const langKeysSet = new Set(fileKeys[lang][file]); - const missingKeys = allKeysAcrossLanguages.filter((key) => !langKeysSet.has(key)); - - return [lang, missingKeys]; - }) - ); - - return Object.entries(missingKeysByLang).some(([, missing]) => missing.length > 0); - }).length - }; + const validationSummary = { + totalFiles: allFiles.length, + totalLanguages: languages.length, + filesWithIssues: filesWithIssues.size + };Also applies to: 154-171, 177-196
scripts/build-meetings.ts (3)
47-53: Use least-privilege scope: calendar.readonlyRead-only is sufficient here and safer than full calendar access.
- auth = new google.auth.GoogleAuth({ - scopes: ['https://www.googleapis.com/auth/calendar'], + auth = new google.auth.GoogleAuth({ + scopes: ['https://www.googleapis.com/auth/calendar.readonly'], credentials: JSON.parse(process.env.CALENDAR_SERVICE_ACCOUNT) });
69-73: Ensure consistent ordering and recurrence expansionInclude singleEvents and orderBy for stable results and expanded recurring instances.
const eventsList = await calendar.events.list({ calendarId: process.env.CALENDAR_ID, timeMax, - timeMin + timeMin, + singleEvents: true, + orderBy: 'startTime' });
86-101: Handle all-day events (start.date) in addition to start.dateTimeAvoid throwing on all-day events; fallback to start.date when dateTime is absent.
- if (!e.start || !e.start.dateTime) { - throw new CustomError('start.dateTime is missing in the event', { + if (!e.start || (!e.start.dateTime && !e.start.date)) { + throw new CustomError('start.dateTime/start.date is missing in the event', { category: 'validation', operation: 'buildMeetings', detail: `Event missing start.dateTime: ${e.summary || 'Unknown event'}` }); } @@ - date: new Date(e.start.dateTime) + date: new Date(e.start.dateTime ?? e.start.date as string) };tests/tools/tools-object.test.ts (2)
165-181: Clear mocks between tests to prevent cross-test leakageAdd jest.clearAllMocks() in beforeEach so logger expectations aren’t polluted by prior tests.
beforeEach(() => { - axiosMock.get.mockClear(); + axiosMock.get.mockClear(); + jest.clearAllMocks(); });
299-329: Always restore Promise.all with finally to avoid flakinessEnsure restoration runs even if assertions fail.
- // Mock Promise.all to throw a non-Error exception - const originalPromiseAll = Promise.all; - - Promise.all = jest.fn().mockRejectedValue('Non-error exception'); - - try { - await convertTools(mockData); - expect(true).toBe(false); // Should not reach here - } catch (error) { - expect(error).toBeInstanceOf(Error); - } - - expect(logger.error).toHaveBeenCalledWith( - 'Error processing tools', - expect.objectContaining({ - error: expect.any(Error) - }) - ); - - // Restore Promise.all - Promise.all = originalPromiseAll; + const originalPromiseAll = Promise.all; + Promise.all = jest.fn().mockRejectedValue('Non-error exception'); + try { + await convertTools(mockData); + expect(true).toBe(false); // Should not reach here + } catch (error) { + expect(error).toBeInstanceOf(Error); + expect(logger.error).toHaveBeenCalledWith( + 'Error processing tools', + expect.objectContaining({ error: expect.any(Error) }) + ); + } finally { + Promise.all = originalPromiseAll; + }scripts/build-newsroom-videos.ts (2)
63-71: Typo in stage: use 'received_data_validation'Minor polish for consistent diagnostics.
- stage: 'recieved_data_validation', + stage: 'received_data_validation',
91-101: Align error model with CustomError used elsewhere in the PRFor consistency and richer categorization, wrap and rethrow via CustomError (or at least logger.error here and throw).
- } catch (err) { - const error = new Error(`Failed to build newsroom videos: ${(err as Error).message}`); - (error as any).context = { - operation: (err as any)?.context?.operation || 'buildNewsroomVideos', - stage: (err as any)?.context?.stage || 'main_execution', - writePath, - errorMessage: (err as Error).message, - errorStack: (err as Error).stack?.split('\n').slice(0, 3).join('\n'), - nestedContext: (err as any)?.context || null - }; - throw error; - } + } catch (err) { + // If CustomError is available in this module, prefer: + // throw CustomError.fromError(err, { category: 'api', operation: 'buildNewsroomVideos', detail: `writePath: ${writePath}` }); + // Otherwise, at least log before rethrowing: + logger.error('Failed to build newsroom videos', { + error: err instanceof Error ? err : new Error(String(err)), + operation: (err as any)?.context?.operation || 'buildNewsroomVideos', + stage: (err as any)?.context?.stage || 'main_execution', + writePath + }); + throw err; + }tests/check-locales.test.ts (5)
1-1: Prefer namespace import for fs to avoid ESM/CJS mock shape pitfalls.Default-importing fs with a plain object mock can be flaky under ESM. Use a namespace import and keep the mock minimal and compatible.
-import fs from 'fs'; +import * as fs from 'fs'; -jest.mock('fs', () => ({ - readdirSync: jest.fn(), - statSync: jest.fn(), - readFileSync: jest.fn() -})); +jest.mock('fs', () => ({ + readdirSync: jest.fn(), + statSync: jest.fn(), + readFileSync: jest.fn() +})); // Get the mocked fs module with proper typing -const mockedFs = fs as jest.Mocked<typeof fs>; +const mockedFs = fs as jest.Mocked<typeof import('fs')>;Also applies to: 14-18, 31-33
35-41: Restore the process.exit spy to avoid test pollution.beforeEach(() => { jest.clearAllMocks(); }); + +afterAll(() => { + mockExit.mockRestore(); +});
129-129: Make thrown-message assertions less brittle.Use regex matching so minor wording changes don’t break tests.
-expect(() => validateLocales()).toThrow('Some translation keys are missing'); +expect(() => validateLocales()).toThrow(/translation keys are missing/i);-expect(() => validateLocales()).toThrow('Error validating locales'); +expect(() => validateLocales()).toThrow(/validating locales/i);-expect(() => validateLocales()).toThrow('No language directories found in'); +expect(() => validateLocales()).toThrow(/no language directories/i);-expect(() => validateLocales()).toThrow('String error'); +expect(() => validateLocales()).toThrow(/string error/i);-expect(() => validateLocales()).toThrow('Validation failed'); +expect(() => validateLocales()).toThrow(/validation failed/i);-expect(() => validateLocales()).toThrow('String error during validation'); +expect(() => validateLocales()).toThrow(/string error during validation/i);Also applies to: 150-150, 168-168, 264-264, 276-276, 458-458
259-266: Test name says “non-Error” but throws an Error. Align the fixture.-mockedFs.readdirSync.mockImplementationOnce(() => { - throw new Error('String error'); -}); +mockedFs.readdirSync.mockImplementationOnce(() => { + // eslint-disable-next-line @typescript-eslint/no-throw-literal + throw 'String error'; +});
500-507: Add a minimal assertion to prove the “filter files with issues” path.-try { - validateLocales(); -} catch (error) { - // Expected to throw -} +expect(() => validateLocales()).toThrow();
| testMatch: [ | ||
| '**/tests/**/*.test.*', | ||
| '**/npm/integrationTests/**/*.test.*', | ||
| '!**/netlify/**/*.test.*' | ||
| ], | ||
| moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'], | ||
| moduleNameMapper: { | ||
| '^@/(.*)$': '<rootDir>/$1' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid negation in testMatch; move exclusions to testPathIgnorePatterns.
Jest’s testMatch is for positive globs; use testPathIgnorePatterns for excludes to prevent accidental test discovery.
Apply:
testMatch: [
- '**/tests/**/*.test.*',
- '**/npm/integrationTests/**/*.test.*',
- '!**/netlify/**/*.test.*'
+ '**/tests/**/*.test.*',
+ '**/npm/integrationTests/**/*.test.*'
],
+ testPathIgnorePatterns: ['<rootDir>/netlify/'],🤖 Prompt for AI Agents
In jest.config.js around lines 17 to 25, the testMatch array contains a negated
glob which should not be used for exclusions; remove the
'!**/netlify/**/*.test.*' entry from testMatch and instead add an appropriate
pattern to testPathIgnorePatterns (for example a pattern matching /netlify/ or
'**/netlify/**') so testMatch only contains positive globs and exclusions are
handled via testPathIgnorePatterns.
| if (!targetYear) { | ||
| const yearsList = fs | ||
| .readdirSync(financeDir) | ||
| // Filter out any files that are not numbers | ||
| .filter((file) => !Number.isNaN(parseFloat(file))) | ||
| // Sort the years in descending order | ||
| .sort((a, b) => parseFloat(b) - parseFloat(a)); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Robustly detect year directories (avoid parseFloat pitfalls and non-directories).
parseFloat will accept names like "2024-old" and even "2024.json". Filter by directory and a strict 4-digit year.
- const yearsList = fs
- .readdirSync(financeDir)
- // Filter out any files that are not numbers
- .filter((file) => !Number.isNaN(parseFloat(file)))
- // Sort the years in descending order
- .sort((a, b) => parseFloat(b) - parseFloat(a));
+ const yearsList = fs
+ .readdirSync(financeDir, { withFileTypes: true })
+ .filter((d) => d.isDirectory() && /^\d{4}$/.test(d.name))
+ .map((d) => d.name)
+ .sort((a, b) => Number(b) - Number(a));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!targetYear) { | |
| const yearsList = fs | |
| .readdirSync(financeDir) | |
| // Filter out any files that are not numbers | |
| .filter((file) => !Number.isNaN(parseFloat(file))) | |
| // Sort the years in descending order | |
| .sort((a, b) => parseFloat(b) - parseFloat(a)); | |
| if (!targetYear) { | |
| const yearsList = fs | |
| .readdirSync(financeDir, { withFileTypes: true }) | |
| .filter((d) => d.isDirectory() && /^\d{4}$/.test(d.name)) | |
| .map((d) => d.name) | |
| .sort((a, b) => Number(b) - Number(a)); |
🤖 Prompt for AI Agents
In npm/runners/build-finance-info-list-runner.ts around lines 42 to 49, the code
uses parseFloat on filenames which wrongly accepts names like "2024-old" or
"2024.json" and doesn't ensure entries are directories; replace the current
readdirSync/filter/sort chain with readdirSync(financeDir, { withFileTypes: true
}) then filter Dirent objects by isDirectory() and name.match(/^\d{4}$/) to only
keep strict 4-digit year directories, then sort by numeric value descending
(e.g., (a, b) => Number(b) - Number(a)).
| // Read the tools files | ||
| const automatedTools: ToolsListObject = JSON.parse(fs.readFileSync(automatedToolsPath, 'utf8')); | ||
| const manualTools: ToolsListObject = JSON.parse(fs.readFileSync(manualToolsPath, 'utf8')); | ||
|
|
||
| await combineTools(automatedTools, manualTools, toolsPath, tagsPath); | ||
| } catch (error) { |
There was a problem hiding this comment.
Runner bypasses core buildTools flow—calls combineTools directly.
This skips fetching/converting automated tools (getData/convertTools) and can silently produce stale outputs. The PR also exports buildTools specifically for runners. Switch to invoking buildTools with resolved paths.
Apply:
-import fs from 'fs';
+// no fs sync reads needed
-import { combineTools } from '@/scripts/tools/combine-tools';
+import { buildTools } from '@/scripts/build-tools';
-import type { ToolsListObject } from '@/types/scripts/tools';
- // Read the tools files
- const automatedTools: ToolsListObject = JSON.parse(fs.readFileSync(automatedToolsPath, 'utf8'));
- const manualTools: ToolsListObject = JSON.parse(fs.readFileSync(manualToolsPath, 'utf8'));
-
- await combineTools(automatedTools, manualTools, toolsPath, tagsPath);
+ await buildTools(automatedToolsPath, manualToolsPath, toolsPath, tagsPath);🤖 Prompt for AI Agents
In npm/runners/build-tools-runner.ts around lines 38 to 43, the runner reads and
JSON.parses the tool files then calls combineTools directly which bypasses the
core buildTools flow (getData/convertTools) and can produce stale outputs;
replace the direct file reads and combineTools call with a call to the exported
buildTools function using the resolved paths (automatedToolsPath,
manualToolsPath, toolsPath, tagsPath) so the canonical fetching/conversion
pipeline is used; remove the direct fs.readFileSync/JSON.parse usage and ensure
you await buildTools(...) and pass any required options so the runner uses the
same logic as other callers.
| // Create or enhance the error with full context | ||
| const customError = | ||
| error instanceof CustomError | ||
| ? error.updateContext({ | ||
| operation: 'runBuildTools', | ||
| detail: `Tools build failed with paths: automated=${options.automatedToolsPath}, manual=${options.manualToolsPath}` | ||
| }) | ||
| : CustomError.fromError(error, { | ||
| category: 'script', | ||
| operation: 'runBuildTools', | ||
| detail: `Build tools runner failed with paths: automated=${options.automatedToolsPath}, manual=${options.manualToolsPath}` | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Error context should include resolved paths (all four).
Using options.* can be undefined and omits tools/tags. Use the resolved variables for better diagnostics.
- detail: `Tools build failed with paths: automated=${options.automatedToolsPath}, manual=${options.manualToolsPath}`
+ detail: `Tools build failed with paths: automated=${automatedToolsPath}, manual=${manualToolsPath}, tools=${toolsPath}, tags=${tagsPath}`
...
- detail: `Build tools runner failed with paths: automated=${options.automatedToolsPath}, manual=${options.manualToolsPath}`
+ detail: `Build tools runner failed with paths: automated=${automatedToolsPath}, manual=${manualToolsPath}, tools=${toolsPath}, tags=${tagsPath}`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Create or enhance the error with full context | |
| const customError = | |
| error instanceof CustomError | |
| ? error.updateContext({ | |
| operation: 'runBuildTools', | |
| detail: `Tools build failed with paths: automated=${options.automatedToolsPath}, manual=${options.manualToolsPath}` | |
| }) | |
| : CustomError.fromError(error, { | |
| category: 'script', | |
| operation: 'runBuildTools', | |
| detail: `Build tools runner failed with paths: automated=${options.automatedToolsPath}, manual=${options.manualToolsPath}` | |
| }); | |
| // Create or enhance the error with full context | |
| const customError = | |
| error instanceof CustomError | |
| ? error.updateContext({ | |
| operation: 'runBuildTools', | |
| detail: `Tools build failed with paths: automated=${automatedToolsPath}, manual=${manualToolsPath}, tools=${toolsPath}, tags=${tagsPath}` | |
| }) | |
| : CustomError.fromError(error, { | |
| category: 'script', | |
| operation: 'runBuildTools', | |
| detail: `Build tools runner failed with paths: automated=${automatedToolsPath}, manual=${manualToolsPath}, tools=${toolsPath}, tags=${tagsPath}` | |
| }); |
🤖 Prompt for AI Agents
In npm/runners/build-tools-runner.ts around lines 44 to 56, the error context
currently uses options.* which can be undefined and omits tools/tags; replace
those with the resolved path variables (e.g., resolvedAutomatedToolsPath,
resolvedManualToolsPath, resolvedAutomatedToolsTagsPath,
resolvedManualToolsTagsPath) so the CustomError.updateContext and
CustomError.fromError detail include all four resolved paths and tags; update
both branches to build the detail string using those resolved variables to
ensure complete diagnostics.
| try { | ||
| await writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json')); | ||
| } catch (err) { | ||
| throw CustomError.fromError(err, { | ||
| category: 'script', | ||
| operation: 'buildAdoptersList', | ||
| detail: `Failed to convert adopters.yml to JSON at ${resolve(currentDirPath, '../../config', 'adopters.json')}` | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resolve the input YAML path; current relative path may break under different CWDs.
Input is passed as 'config/adopters.yml' (relative), while output is resolved. Resolve both to avoid environment-dependent failures.
-export async function buildAdoptersList() {
- const currentFilePath = fileURLToPath(import.meta.url);
- const currentDirPath = dirname(currentFilePath);
-
- try {
- await writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json'));
+export async function buildAdoptersList() {
+ const currentFilePath = fileURLToPath(import.meta.url);
+ const currentDirPath = dirname(currentFilePath);
+ const inputPath = resolve(currentDirPath, '../../config', 'adopters.yml');
+ const outputPath = resolve(currentDirPath, '../../config', 'adopters.json');
+
+ try {
+ await writeJSON(inputPath, outputPath);
} catch (err) {
throw CustomError.fromError(err, {
category: 'script',
operation: 'buildAdoptersList',
- detail: `Failed to convert adopters.yml to JSON at ${resolve(currentDirPath, '../../config', 'adopters.json')}`
+ detail: `Failed to convert adopters.yml to JSON at ${outputPath}`
});
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| await writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json')); | |
| } catch (err) { | |
| throw CustomError.fromError(err, { | |
| category: 'script', | |
| operation: 'buildAdoptersList', | |
| detail: `Failed to convert adopters.yml to JSON at ${resolve(currentDirPath, '../../config', 'adopters.json')}` | |
| }); | |
| } | |
| export async function buildAdoptersList() { | |
| const currentFilePath = fileURLToPath(import.meta.url); | |
| const currentDirPath = dirname(currentFilePath); | |
| const inputPath = resolve(currentDirPath, '../../config', 'adopters.yml'); | |
| const outputPath = resolve(currentDirPath, '../../config', 'adopters.json'); | |
| try { | |
| await writeJSON(inputPath, outputPath); | |
| } catch (err) { | |
| throw CustomError.fromError(err, { | |
| category: 'script', | |
| operation: 'buildAdoptersList', | |
| detail: `Failed to convert adopters.yml to JSON at ${outputPath}` | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In scripts/adopters/index.ts around lines 21 to 29, the code writes JSON from a
relative input path 'config/adopters.yml' while resolving the output path, which
can fail when the process CWD differs; change the input path to be resolved the
same way (e.g., resolve(currentDirPath, '../../config', 'adopters.yml')) before
passing it to writeJSON so both input and output use absolute/resolved paths and
the operation is deterministic across environments.
| /** | ||
| * Log an error - accepts either a string message with CustomError, or just a string with meta | ||
| */ | ||
| error: (message: string, errorOrMeta?: CustomError | Record<string, unknown>) => { | ||
| if (errorOrMeta instanceof CustomError) { | ||
| // String message with CustomError - log as structured data | ||
| const errorData = extractErrorData(errorOrMeta); | ||
|
|
||
| baseLogger.error(message, { | ||
| error: errorOrMeta, | ||
| errorData | ||
| }); | ||
| } else { | ||
| // Regular error logging | ||
| baseLogger.error(message, errorOrMeta as Record<string, unknown>); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
logger.error drops plain Error objects; ensure generic Errors are logged with stack.
Passing logger.error('msg', new Error(...)) currently serializes to empty meta and loses the stack. Handle Error explicitly and preserve existing CustomError flow.
/**
- * Log an error - accepts either a string message with CustomError, or just a string with meta
+ * Log an error - accepts CustomError, generic Error, or a meta object (optionally containing `error`)
*/
- error: (message: string, errorOrMeta?: CustomError | Record<string, unknown>) => {
- if (errorOrMeta instanceof CustomError) {
+ error: (message: string, errorOrMeta?: CustomError | Error | Record<string, unknown>) => {
+ if (errorOrMeta instanceof CustomError) {
// String message with CustomError - log as structured data
const errorData = extractErrorData(errorOrMeta);
baseLogger.error(message, {
error: errorOrMeta,
errorData
});
- } else {
- // Regular error logging
- baseLogger.error(message, errorOrMeta as Record<string, unknown>);
- }
+ } else if (errorOrMeta instanceof Error) {
+ // Generic Error: include under `error` so formatter prints stack
+ baseLogger.error(message, { error: errorOrMeta });
+ } else if (errorOrMeta && typeof errorOrMeta === 'object' && 'error' in (errorOrMeta as Record<string, unknown>)) {
+ // Meta already provides an `error` field
+ baseLogger.error(message, errorOrMeta as Record<string, unknown>);
+ } else {
+ // Plain metadata
+ baseLogger.error(message, errorOrMeta as Record<string, unknown>);
+ }
}Also applies to: 73-94
🤖 Prompt for AI Agents
In scripts/helpers/logger.ts around lines 107-124 (and similarly 73-94), the
logger.error path currently treats non-CustomError meta generically and drops
plain Error objects; modify the branch to explicitly detect instances of Error
(but not CustomError) and transform them into a meta object that preserves at
minimum error.message and error.stack (and the original Error under a key like
error) before passing to baseLogger.error, while leaving the existing
CustomError flow unchanged.
| * This function requires the GITHUB_TOKEN environment variable for authorization. It retrieves data in pages (up to 50 | ||
| * items per page) and pauses 1 second between requests to respect GitHub's rate limits. All pages are aggregated into | ||
| * a single result before being returned. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pagination logic is incorrect; do not use incomplete_results to page
GitHub’s incomplete_results isn’t a pagination flag. Use Link: rel="next" or items.length to paginate, and dedupe by a stable key (e.g., repository full_name + path), not object identity.
- const allItems = new Set();
+ const items: any[] = [];
+ const seen = new Set<string>();
+ const keyOf = (item: any) => `${item.repository?.full_name ?? ''}/${item.path ?? ''}`;
+ const pushUnique = (arr: any[]) => {
+ for (const it of arr) {
+ const k = keyOf(it);
+ if (!seen.has(k)) {
+ seen.add(k);
+ items.push(it);
+ }
+ }
+ };
@@
- const maxPerPage = 50;
- const getReqUrl = (PerPage: number, pageNo: number) =>
- `https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=${PerPage}&page=${pageNo}`;
+ const maxPerPage = 100;
+ const getReqUrl = (perPage: number, pageNo: number) =>
+ `https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=${perPage}&page=${pageNo}`;
@@
- const result = await axios.get(getReqUrl(maxPerPage, page), {
- headers
- });
-
- incompleteResult = result.data.incomplete_results || false;
-
- result.data.items.forEach((item: any) => {
- allItems.add(item);
- });
-
- while (incompleteResult) {
+ const first = await axios.get(getReqUrl(maxPerPage, page), { headers });
+ pushUnique(first.data.items);
+ let hasNext = Boolean((first.headers.link || '').includes('rel="next"'));
+
+ while (hasNext) {
page++;
logger.info(`Fetching page: ${page}`);
// pause for 1 second to avoid rate limiting
await pause(1000);
- const nextPageData = await axios.get(getReqUrl(maxPerPage, page), {
- headers
- });
+ const nextPageData = await axios.get(getReqUrl(maxPerPage, page), { headers });
@@
- data.items.forEach((item: any) => {
- allItems.add(item);
- });
-
- incompleteResult = data.incomplete_results || false;
+ pushUnique(data.items);
+ hasNext = Boolean((nextPageData.headers.link || '').includes('rel="next"'));
}
- result.data.items = [...allItems];
-
- return result.data.items;
+ return items as unknown as ToolsData;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/tools/extract-tools-github.ts around lines 15 to 17, the pagination
currently relies on GitHub's incomplete_results which is not a pagination flag;
replace that logic to follow the Link: rel="next" header (or stop when the
returned items array length is less than the requested per_page) to determine
when to stop requesting pages, and continue fetching until no next link is
present; also deduplicate aggregated results using a stable key such as
repository_full_name + path (or another unique identifier) instead of object
identity, and retain the existing rate-limit pause and per_page settings while
ensuring each page's items are appended and de-duplicated by that stable key.
| const isValid = validate(jsonToolFileContent); | ||
|
|
||
| if (isValid) { | ||
| const repositoryUrl = tool.repository.html_url; | ||
| const repoDescription = tool.repository.description; | ||
| const isAsyncAPIrepo = tool.repository.owner.login === 'asyncapi'; | ||
| const toolObject = await createToolObject( | ||
| jsonToolFileContent, | ||
| jsonToolFileContent as unknown as AsyncAPITool, | ||
| repositoryUrl, | ||
| repoDescription, | ||
| isAsyncAPIrepo | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Narrow types after validation; drop double cast and keep createToolObject signature clean.
After isValid, safely narrow jsonToolFileContent and pass strongly-typed data.
- const isValid = validate(jsonToolFileContent);
+ const isValid = validate(jsonToolFileContent);
if (isValid) {
const repositoryUrl = tool.repository.html_url;
const repoDescription = tool.repository.description;
const isAsyncAPIrepo = tool.repository.owner.login === 'asyncapi';
- const toolObject = await createToolObject(
- jsonToolFileContent as unknown as AsyncAPITool,
- repositoryUrl,
- repoDescription,
- isAsyncAPIrepo
- );
+ const toolData = jsonToolFileContent as AsyncAPITool;
+ const toolObject = await createToolObject(
+ toolData,
+ repositoryUrl,
+ repoDescription,
+ isAsyncAPIrepo
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isValid = validate(jsonToolFileContent); | |
| if (isValid) { | |
| const repositoryUrl = tool.repository.html_url; | |
| const repoDescription = tool.repository.description; | |
| const isAsyncAPIrepo = tool.repository.owner.login === 'asyncapi'; | |
| const toolObject = await createToolObject( | |
| jsonToolFileContent, | |
| jsonToolFileContent as unknown as AsyncAPITool, | |
| repositoryUrl, | |
| repoDescription, | |
| isAsyncAPIrepo | |
| ); | |
| const isValid = validate(jsonToolFileContent); | |
| if (isValid) { | |
| const repositoryUrl = tool.repository.html_url; | |
| const repoDescription = tool.repository.description; | |
| const isAsyncAPIrepo = tool.repository.owner.login === 'asyncapi'; | |
| const toolData = jsonToolFileContent as AsyncAPITool; | |
| const toolObject = await createToolObject( | |
| toolData, | |
| repositoryUrl, | |
| repoDescription, | |
| isAsyncAPIrepo | |
| ); |
🤖 Prompt for AI Agents
In scripts/tools/tools-object.ts around lines 118 to 129, after
validate(jsonToolFileContent) returns true you should narrow jsonToolFileContent
to AsyncAPITool instead of using the double cast; either change validate's
return type to a type predicate (validate(x): x is AsyncAPITool) or assert once
into a local const (e.g., const toolData = jsonToolFileContent as AsyncAPITool)
and pass toolData into createToolObject, removing the "as unknown as" double
cast so createToolObject receives a properly typed AsyncAPITool.
| // Restore the mock | ||
| mockWriteJSON.mockRestore(); | ||
| }); |
There was a problem hiding this comment.
mockRestore is invalid on jest.fn mocks; use mockReset or rely on clearAllMocks.
Current call can throw at runtime and fail the test.
- // Restore the mock
- mockWriteJSON.mockRestore();
+ // No-op: mocks are cleared in beforeEach. Optionally:
+ // (writeJSON as jest.MockedFunction<typeof writeJSON>).mockReset();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Restore the mock | |
| mockWriteJSON.mockRestore(); | |
| }); | |
| afterEach(() => { | |
| // No-op: mocks are cleared in beforeEach. Optionally: | |
| // (writeJSON as jest.MockedFunction<typeof writeJSON>).mockReset(); | |
| }); |
🤖 Prompt for AI Agents
In tests/finance/index.test.ts around lines 92 to 94, the test calls
mockWriteJSON.mockRestore() which is invalid for jest.fn() mocks and can throw;
replace that call with mockWriteJSON.mockReset() (or mockWriteJSON.mockClear()
if you only need to clear calls) or remove it and rely on
jest.clearAllMocks()/jest.resetAllMocks() in your test setup/teardown so the
mock is safely cleared without runtime errors.
| const context: ErrorContext = { | ||
| category: 'general', | ||
| ...(isCustomError ? error.context : {}), | ||
| ...additionalContext, | ||
| originalError: isCustomError ? error.context.originalError : error, | ||
| timestamp: new Date().toISOString(), | ||
| stackTrace: existingStackTrace | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve originalError when wrapping an existing CustomError.
Avoid dropping the immediate wrapper error if originalError is undefined.
- originalError: isCustomError ? error.context.originalError : error,
+ originalError: isCustomError ? (error.context.originalError ?? error) : error,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const context: ErrorContext = { | |
| category: 'general', | |
| ...(isCustomError ? error.context : {}), | |
| ...additionalContext, | |
| originalError: isCustomError ? error.context.originalError : error, | |
| timestamp: new Date().toISOString(), | |
| stackTrace: existingStackTrace | |
| }; | |
| const context: ErrorContext = { | |
| category: 'general', | |
| ...(isCustomError ? error.context : {}), | |
| ...additionalContext, | |
| originalError: isCustomError ? (error.context.originalError ?? error) : error, | |
| timestamp: new Date().toISOString(), | |
| stackTrace: existingStackTrace | |
| }; |
🤖 Prompt for AI Agents
In types/errors/CustomError.ts around lines 75 to 82, when building the context
for a wrapped error the code uses originalError: isCustomError ?
error.context.originalError : error which can drop the immediate wrapper if
error.context.originalError is undefined; change the assignment to preserve the
immediate wrapper by using the existing originalError when present otherwise
falling back to the current error (e.g. originalError: isCustomError ?
(error.context.originalError ?? error) : error), keep the rest of the context
merging behavior the same, and ensure types remain compatible.
Description
Related issue(s)
Summary by CodeRabbit