refactor: standardize runtime guard patterns with lodash-es#154
Conversation
| const credits = updatedAccount.quota?.ai_credits?.credits; | ||
| if (typeof credits === 'number') { | ||
| if (isNumber(credits)) { | ||
| toast({ | ||
| title: t('cloud.toast.quotaRefreshed'), | ||
| description: `AI credits: $${credits.toFixed(2)}`, |
There was a problem hiding this comment.
Potential issue with formatting credits value:
The code uses credits.toFixed(2) without verifying that credits is a valid number. If credits is NaN, undefined, or otherwise invalid, this could result in misleading output or a runtime error. Consider adding a stricter check:
if (isNumber(credits) && !isNaN(credits)) {
toast({
title: t('cloud.toast.quotaRefreshed'),
description: `AI credits: $${credits.toFixed(2)}`,
});
return;
}This ensures only valid numbers are formatted and displayed.
| if (isNumber(collisionPadding)) { | ||
| if (collisionPadding < 0) { | ||
| throw new Error('collisionPadding must be a non-negative number'); | ||
| } |
There was a problem hiding this comment.
Error Handling Issue: Throwing an error (throw new Error) for invalid collisionPadding values inside a React component can cause the entire component tree to unmount if not caught, leading to poor user experience. Instead, consider logging a warning or defaulting to a safe value:
if (isNumber(collisionPadding) && collisionPadding < 0) {
console.warn('collisionPadding must be a non-negative number');
collisionPadding = 8; // fallback to default
}This approach prevents disruptive errors and maintains application stability.
| } else if (isObjectLike(collisionPadding)) { | ||
| for (const key of Object.keys(collisionPadding)) { | ||
| if ((collisionPadding as any)[key] < 0) { | ||
| throw new Error(`collisionPadding.${key} must be a non-negative number`); |
There was a problem hiding this comment.
Type Validation Issue: The use of isObjectLike from lodash for validating collisionPadding may allow non-plain objects (such as arrays or Date objects), which could lead to unexpected behavior. To ensure only plain objects with the expected keys are accepted, use a stricter check:
if (typeof collisionPadding === 'object' && collisionPadding !== null && !Array.isArray(collisionPadding)) {
// proceed with validation
}This ensures the prop conforms to the intended shape and prevents runtime errors.
|
|
||
| const normalized: DeviceProfileVersion[] = []; | ||
| for (const item of value) { | ||
| if (!isRecord(item)) { | ||
| if (!isPlainObject(item)) { | ||
| continue; | ||
| } | ||
| const itemRecord = item as Record<string, unknown>; | ||
|
|
||
| const profile = normalizeDeviceProfile(item.profile); | ||
| const profile = normalizeDeviceProfile(itemRecord.profile); | ||
| if (!profile) { | ||
| continue; | ||
| } | ||
|
|
||
| const id = typeof item.id === 'string' && item.id.length > 0 ? item.id : uuidv4(); | ||
| const createdAtCandidate = item.createdAt; | ||
| const id = isString(itemRecord.id) && itemRecord.id.length > 0 ? itemRecord.id : uuidv4(); | ||
| const createdAtCandidate = itemRecord.createdAt; | ||
| const createdAt = | ||
| typeof createdAtCandidate === 'number' && Number.isFinite(createdAtCandidate) | ||
| isNumber(createdAtCandidate) && Number.isFinite(createdAtCandidate) | ||
| ? Math.floor(createdAtCandidate) | ||
| : Math.floor(Date.now() / 1000); | ||
| const label = typeof item.label === 'string' && item.label.length > 0 ? item.label : 'legacy'; | ||
| const isCurrent = item.isCurrent === true; | ||
| const label = | ||
| isString(itemRecord.label) && itemRecord.label.length > 0 ? itemRecord.label : 'legacy'; | ||
| const isCurrent = itemRecord.isCurrent === true; | ||
|
|
||
| normalized.push({ | ||
| id, |
There was a problem hiding this comment.
In normalizeDeviceHistory, invalid entries (non-objects or entries with invalid/missing profiles) are silently skipped. This could mask underlying data issues and make debugging difficult if device history is unexpectedly incomplete.
Recommendation: Consider logging or counting skipped entries for diagnostics, or at least returning information about how many entries were ignored. For example:
if (!profile) {
logger.warn(`Skipped device history entry with invalid profile at index ${i}`);
continue;
}This will help with maintainability and troubleshooting.
| for (const key of KEYS_TO_BACKUP) { | ||
| if (key in backup.data) { | ||
| const value = backup.data[key]; | ||
| const stringValue = typeof value === 'string' ? value : JSON.stringify(value); | ||
| const stringValue = isString(value) ? value : JSON.stringify(value); | ||
| tx.insert(itemTable) | ||
| .values({ key, value: stringValue }) | ||
| .onConflictDoUpdate({ |
There was a problem hiding this comment.
Issue: Lack of Data Validation During Restore
When restoring backup data, the code directly writes values from the backup into the database without validating their structure or content. If the backup file is corrupted or has been tampered with, this could result in invalid or malicious data being persisted, potentially leading to application errors or security vulnerabilities.
Recommendation:
Before writing each value to the database, validate that it conforms to the expected schema for that key. For example, use a schema validator or type guard to ensure the data is well-formed:
if (validateKeyValue(key, value)) {
// proceed with insert
} else {
logger.warn(`Invalid data for key: ${key}, skipping restore for this key.`);
continue;
}Implement validateKeyValue to check the structure and type of each value according to your application's requirements.
| if (res.raw.writableEnded) { | ||
| return; | ||
| } | ||
| const payload = typeof chunk === 'string' ? chunk : String(chunk ?? ''); | ||
| const payload = isString(chunk) ? chunk : String(chunk ?? ''); | ||
| res.raw.write(payload); | ||
| }, | ||
| error: (error) => { |
There was a problem hiding this comment.
In the SSE next handler, the chunk is stringified if not a string, which could result in unexpected output if the chunk is a complex object (e.g., an object with circular references or large nested structures). This could cause client-side parsing issues or performance problems.
Recommendation: Ensure that the chunk is properly serialized to a format expected by the client (e.g., JSON) and consider validating the chunk's structure before writing it to the response. If the chunk is not serializable, log an error and skip sending it.
| private buildLockoutKey(accountId: string, model?: string): string { | ||
| if (model && model.trim() !== '') { | ||
| if (!isEmpty(model?.trim() ?? '')) { | ||
| return `${accountId}:${model}`; | ||
| } | ||
| return accountId; |
There was a problem hiding this comment.
Potential ambiguity in key construction:
The buildLockoutKey method uses isEmpty(model?.trim() ?? '') to determine whether to include the model in the key. If the model is a whitespace-only string, it is treated as empty, which could lead to ambiguous keys (e.g., accountId: vs accountId). Consider explicitly handling whitespace-only model names or rejecting them to avoid ambiguity:
if (model && model.trim() !== '') {
return `${accountId}:${model.trim()}`;
}
return accountId;| const retryAfterSec = Math.max(2, Math.ceil((resetTimeMs - Date.now()) / 1000)); | ||
| const key = model && model.trim() !== '' ? this.buildLockoutKey(accountId, model) : accountId; | ||
| const key = !isEmpty(model?.trim() ?? '') ? this.buildLockoutKey(accountId, model) : accountId; | ||
| this.lockoutByKey.set(key, { | ||
| resetTimeMs, | ||
| retryAfterSec, |
There was a problem hiding this comment.
Hardcoded minimum retryAfterSec may lack flexibility:
The setLockoutUntil method enforces a minimum retryAfterSec of 2 seconds. While this prevents immediate retries, it may not be optimal for all use cases. Consider making this value configurable or documenting the rationale for this choice to improve maintainability and adaptability.
| let pendingUserAgentResolution: Promise<UserAgentResolution> | null = null; | ||
|
|
There was a problem hiding this comment.
Potential Data Race with Shared Mutable State
The use of let cachedUserAgentResolution and let pendingUserAgentResolution as module-level variables introduces a risk of data races in concurrent environments. If multiple asynchronous calls to resolveRequestUserAgent or related functions occur simultaneously, it is possible for the cache or pending promise to be set or read inconsistently, leading to redundant resolutions or inconsistent results.
Recommendation:
Consider using a concurrency-safe mechanism (such as a mutex or atomic operations) to guard access to these shared variables, or refactor to avoid shared mutable state if possible.
| }, | ||
| }); | ||
|
|
||
| if (typeof response.data === 'string') { | ||
| if (isString(response.data)) { | ||
| return response.data; | ||
| } | ||
|
|
There was a problem hiding this comment.
Inconsistent Response Handling in fetchTextPayload
The code stringifies non-string response.data values before returning them. If the remote endpoint returns an object or other non-string payload, this may result in an unexpected format for downstream consumers, potentially causing subtle bugs or incorrect user agent strings.
Recommendation:
Validate the response type more strictly and consider returning null or throwing an error if the payload is not a string, unless there is a clear use case for stringifying arbitrary data.
## [0.11.1](v0.11.0...v0.11.1) (2026-04-18) ### 🐛 Bug Fixes * add unit label to AI credit display ([6577f1c](6577f1c)) * remaining localization gaps ([#156](#156)) ([7093c98](7093c98)) * remove AI credit refresh time scraping ([3e1e10b](3e1e10b)) ### 📝 Documentation * **repo:** update project docs ([440a934](440a934)) ### ♻️ Code Refactoring * **ipc:** migrate renderer IPC client to official oRPC client ([d0c282d](d0c282d)) * standardize runtime guard patterns with lodash-es ([#154](#154)) ([bc35b96](bc35b96))
Summary
This PR standardizes runtime guard patterns by replacing ad-hoc
typeofchecks and trim-based empty checks withlodash-espredicates.What changed
isString/isNumber/isBoolean/isFunction/isObjectLike/isUndefined.isEmpty(value.trim())where applicable..agents/skills/lodash-es-runtime-guards/SKILL.mdVerification
rg -n "typeof\\s+[^\\n]+(?:===|!==)\\s*'" srcreturns no runtimetypeofcomparisons.npm run type-checkpasses.