refactor(utils): consolidate duplicated helpers onto @sim/utils#5509
Conversation
Replaces ~90 hand-rolled reimplementations of error-message extraction, postgres error-code checks, sleep, Math.random, retry/backoff, object filtering/omission, noop, string truncation, date/time formatting, email normalization, and plain-object type guards with the shared @sim/utils exports. Wires check:utils into CI (test-build.yml) so these patterns don't regress.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Errors & DB: Auth & email: Login, public file OTP/SSO, org invites, billing credits, deployment allow-lists, and related UI now use Formatting & strings: Shared Async & random: Object guards: Reviewed by Cursor Bugbot for commit 3a06e25. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3a06e25. Configure here.
Greptile SummaryThis PR is a comprehensive deduplication refactor across 93 files, consolidating hand-rolled helpers (error extraction, sleep/retry/backoff, string truncation, date/time formatting, email normalization, plain-object guards) onto the shared
Confidence Score: 4/5Safe to merge; the one display-quality regression in schedule timezone labels for non-hardcoded IANA zones is cosmetic and does not affect data or workflow execution. The vast majority of substitutions are mechanically correct and the retry/backoff rework in tools/index.ts handles the changed null-return contract properly at every call site. The only notable regression is in schedules/utils.ts: the deleted local getTimezoneAbbreviation resolved abbreviations for any IANA timezone via Intl.DateTimeFormat, while the replacement from @sim/utils hardcodes 9 zones and returns the raw IANA string for everything else — so users with timezones like Europe/Berlin or America/Toronto will see the full IANA name in schedule descriptions instead of a short abbreviation. apps/sim/lib/workflows/schedules/utils.ts — the getTimezoneAbbreviation substitution degrades display quality for any IANA timezone not in the shared util's 9-entry hardcoded map Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["apps/sim (93 files)"] --> B["@sim/utils"]
C["packages/logger"] --> B
D["packages/workflow-persistence"] --> B
B --> E["errors\ngetErrorMessage / toError / getPostgresErrorCode"]
B --> F["string\nnormalizeEmail / truncate"]
B --> G["retry\nbackoffWithJitter / parseRetryAfter"]
B --> H["formatting\nformatDate / formatDateTime\nformatRelativeTime / getTimezoneAbbreviation"]
B --> I["object\nfilterUndefined / omit / isRecordLike"]
B --> J["helpers\nsleep / noop"]
K[".github/workflows/test-build.yml"] --> L["bun run check:utils\n(CI enforcement)"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["apps/sim (93 files)"] --> B["@sim/utils"]
C["packages/logger"] --> B
D["packages/workflow-persistence"] --> B
B --> E["errors\ngetErrorMessage / toError / getPostgresErrorCode"]
B --> F["string\nnormalizeEmail / truncate"]
B --> G["retry\nbackoffWithJitter / parseRetryAfter"]
B --> H["formatting\nformatDate / formatDateTime\nformatRelativeTime / getTimezoneAbbreviation"]
B --> I["object\nfilterUndefined / omit / isRecordLike"]
B --> J["helpers\nsleep / noop"]
K[".github/workflows/test-build.yml"] --> L["bun run check:utils\n(CI enforcement)"]
|
…execute tests The schedules/execute route previously used Math.random() for jitter delay; this consolidation PR switched it to randomInt() from @sim/utils/random, which is backed by crypto.getRandomValues() rather than Math.random(). The route.test.ts spies on Math.random() no longer had any effect, so jitter became real random delay instead of the deterministic 0ms the tests expect, causing intermittent 10s timeouts in CI.
parseRetryAfter() caps its return value at 30s by default. tools/index.ts compares the parsed Retry-After against a caller-configured maxDelayMs to decide whether to skip a retry entirely -- capping before that comparison silently defeats the skip check whenever maxDelayMs is configured above 30s, since a Retry-After between 30s and maxDelayMs would incorrectly look "within limits" and get retried instead of skipped (caught by Cursor Bugbot). Added an optional maxMs param (default unchanged) so tools/index.ts can request the raw, uncapped value for its own comparison while backoffWithJitter still clamps the actual sleep duration to maxDelayMs. Added a regression test covering maxDelayMs > 30s.
…zones getTimezoneAbbreviation only covered 9 hardcoded IANA zones and returned the raw IANA string for everything else, degrading schedule descriptions for zones like Europe/Berlin or America/Toronto (caught by Greptile). The deleted local implementation in schedules/utils.ts resolved any valid IANA timezone generically via Intl.DateTimeFormat's short timeZoneName. Restore that as a fallback so only genuinely invalid timezone strings return themselves unchanged.
|
Addressed both review findings on this branch:
Also fixed an unrelated CI test failure in |
Summary
@sim/utilshelpers (errors, id/random, retry, object, string, formatting) and fixed every genuine duplicate found.check:utils(existing enforcement script, previously unused in CI) intotest-build.ymlso these patterns can't silently regress.What changed
error instanceof Error ? error.message : fallback→getErrorMessage(); manual.code === '23505'checks →getPostgresErrorCode()(19 files)setTimeoutpromises →sleep();Math.random()→randomFloat/randomInt/generateShortId();apps/sim/tools/index.ts's localcalculateBackoff/parseRetryAfterHeader→backoffWithJitter/parseRetryAfter(already importing sibling@sim/utilshelpers in the same file)const noop = () => {}→ sharednoop;Object.fromEntries(Object.entries(...).filter(...))→filterUndefined/omitinpackages/workflow-persistence,packages/logger(added as a new workspace dependency, zero circularity risk), andapps/sim/providers/utils.tstruncate(), preserving exact visible output (two sites were unconditional before and are now correctly conditional — flagged as intentional behavior fixes)getTimezoneAbbreviation/relative-time ladder/toLocaleDateStringreimplementations (10 files, including two files with an identical copy-pasted helper) →getTimezoneAbbreviation/formatRelativeTime/formatDate/formatDateTime. A few of these are minor visible format changes (e.g. teammates/billing dates move from locale-dependentM/D/YYYYto fixedMMM D, YYYY) in exchange for one consistent format across the app..trim().toLowerCase()→normalizeEmail()at 13+ call sites across auth/invite/billing flowsisRecord/isPlainObjectreimplementations →isRecordLike()(7 sites)Intentionally skipped (documented in code/PR, not silent):
packages/ts-sdkandpackages/cli(standalone published packages, already exempt incheck-utils-enforcement.ts),packages/emcn's email handler (kept dependency-free by design), and a handful of lower-confidence tool-output/chart-axis date formatters that aren't generic UI dates.Test plan
bun run check:utils— passes (was previously failing with ~30 violations, never run in CI)bunx tsc --noEmit -p apps/sim/tsconfig.json— cleanbunx biome check— cleanbun run check:api-validation— passestools/index.test.ts+providers/utils.test.ts+ 5 more files with dedicated coverage — 443 tests passing