Fix voting/commenting#3115
Merged
feruzm merged 6 commits intodevelopmentfrom Feb 18, 2026
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughBumps two dependencies, adds dsteem/Hive date-error detection and diagnostics with consistent TAPOS client usage, enhances waves comment-cache invalidation, disables mobile posting-authorization optimization, centralizes media-picker error reporting, and hardens dsteem error extraction. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App / UI
participant Dhive as dhive Client
participant Hive as Hive API Server
participant SDK as ecency SDK
participant Sentry as Sentry
UI->>Dhive: sendHiveOperations(ops)
Dhive->>Dhive: fetch TAPOS via same client
Dhive->>Hive: broadcast_transaction
alt success
Hive-->>Dhive: success
Dhive-->>UI: success
else date-related error
Hive-->>Dhive: error (trx.expiration / transaction expiration)
Dhive->>Dhive: isDsteemDateError(error) => true
Dhive->>Dhive: getDateErrorDiagnostics() (local time, tz, client head, sdk head, skew)
Dhive->>Sentry: captureException(error + dateDiagnostics) with tag error_type=dsteem_date_error
Dhive-->>UI: return error with diagnostics
else other error
Dhive->>Sentry: captureException(error)
Dhive-->>UI: return error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/providers/sdk/mobilePlatformAdapter.ts (1)
43-44:⚠️ Potential issue | 🔴 Critical
getAccountFullQueryOptionsis used but not imported — TypeScript compile error.Line 43 references
getAccountFullQueryOptions(username)inside thegetUsermethod, but the import from@ecency/sdkis missing. This will fail TypeScript compilation with "Cannot find name 'getAccountFullQueryOptions'".🐛 Proposed fix: restore the missing import
import type { PlatformAdapter } from '@ecency/sdk'; +import { getAccountFullQueryOptions } from '@ecency/sdk';Alternatively, if
getUseris no longer needed by the SDK (e.g. becausehasPostingAuthorizationnow short-circuits before any account lookup), the entiregetUserimplementation could be updated to returnundefinedimmediately:getUser: async (username: string) => { - return queryClient.getQueryData(getAccountFullQueryOptions(username).queryKey); + return undefined; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/sdk/mobilePlatformAdapter.ts` around lines 43 - 44, The getUser implementation calls getAccountFullQueryOptions(username) but that symbol is not imported, causing a TypeScript error; fix by adding an import for getAccountFullQueryOptions from '@ecency/sdk' at the top of the file, or if getUser should no longer perform lookups, change the getUser method (the function named getUser in mobilePlatformAdapter.ts) to short-circuit and return undefined instead of calling getAccountFullQueryOptions/getQueryData—pick one approach and apply it consistently.
🧹 Nitpick comments (2)
src/providers/hive/dhive.ts (1)
124-137:message.includes('expiration')inisDsteemDateErroris too broad.The catch-all
'expiration'substring (line 134) would match unrelated errors — e.g., HiveSigner token-expiry messages, OAuth session expiry — causinggetDateErrorDiagnosticsto make two unnecessary RPC calls and incorrectly tagging those errors asdsteem_date_errorin Sentry. The more specific checks ('trx.expiration','transaction expiration','trx_expiration') already cover the known Hive transaction date error patterns.♻️ Suggested fix
return ( message.includes('trx.expiration') || message.includes('transaction expiration') || - message.includes('expiration') || message.includes('trx_expiration') );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/hive/dhive.ts` around lines 124 - 137, The isDsteemDateError predicate is too broad because the generic message.includes('expiration') causes unrelated errors to be classified as dsteem_date_error and triggers extra RPC calls in getDateErrorDiagnostics; update isDsteemDateError to remove the generic 'expiration' substring check and only check the specific patterns already present (e.g., 'trx.expiration', 'transaction expiration', 'trx_expiration' and the jse_info.code 4030100), so that only known Hive transaction date errors are matched.src/providers/queries/postQueries/wavesQueries.ts (1)
126-145:_injectCommentCacheis a misleading name — the function invalidates rather than injects.Unlike
_injectPostCachewhich directly mutates query data (optimistic update), this function only callsqueryClient.invalidateQueries, which triggers a full re-fetch. The name implies writing something into the cache, which it doesn't do.♻️ Suggested rename
- const _injectCommentCache = (commentPath: string) => { + const _invalidateContainerForComment = (commentPath: string) => {And the call site:
- _injectCommentCache(lastCacheUpdate.postPath); + _invalidateContainerForComment(lastCacheUpdate.postPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/queries/postQueries/wavesQueries.ts` around lines 126 - 145, Function _injectCommentCache is misnamed because it does not write to cache but only invalidates; rename it to reflect behavior (e.g., invalidateCommentWaveCache or invalidateWaveContainerForComment) and update all call sites (places that call _injectCommentCache) accordingly. Inside the renamed function keep the same logic that derives parentPath from _cachedComment, reads wavesIndexCollection.current[parentPath], finds _containerIndex via activePermlinks.indexOf(_containerPermlink) and calls queryClient.invalidateQueries({ queryKey: [QUERIES.WAVES.GET, host, _containerPermlink, _containerIndex] }). Also consider adding a short comment that this is an invalidate-only path to distinguish it from _injectPostCache which performs optimistic cache writes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/dsteemUtils.ts`:
- Line 31: The hard-coded English string in parts.push('You can also try
changing API Server in Settings > Server.'); bypasses i18n; replace that literal
with intl.formatMessage({ id: 'dsteem.date_error.server_hint' }) in the same
function (where other strings use intl.formatMessage) and then add the key
dsteem.date_error.server_hint with translated values to each locale file so
non-English users see the localized hint.
---
Outside diff comments:
In `@src/providers/sdk/mobilePlatformAdapter.ts`:
- Around line 43-44: The getUser implementation calls
getAccountFullQueryOptions(username) but that symbol is not imported, causing a
TypeScript error; fix by adding an import for getAccountFullQueryOptions from
'@ecency/sdk' at the top of the file, or if getUser should no longer perform
lookups, change the getUser method (the function named getUser in
mobilePlatformAdapter.ts) to short-circuit and return undefined instead of
calling getAccountFullQueryOptions/getQueryData—pick one approach and apply it
consistently.
---
Nitpick comments:
In `@src/providers/hive/dhive.ts`:
- Around line 124-137: The isDsteemDateError predicate is too broad because the
generic message.includes('expiration') causes unrelated errors to be classified
as dsteem_date_error and triggers extra RPC calls in getDateErrorDiagnostics;
update isDsteemDateError to remove the generic 'expiration' substring check and
only check the specific patterns already present (e.g., 'trx.expiration',
'transaction expiration', 'trx_expiration' and the jse_info.code 4030100), so
that only known Hive transaction date errors are matched.
In `@src/providers/queries/postQueries/wavesQueries.ts`:
- Around line 126-145: Function _injectCommentCache is misnamed because it does
not write to cache but only invalidates; rename it to reflect behavior (e.g.,
invalidateCommentWaveCache or invalidateWaveContainerForComment) and update all
call sites (places that call _injectCommentCache) accordingly. Inside the
renamed function keep the same logic that derives parentPath from
_cachedComment, reads wavesIndexCollection.current[parentPath], finds
_containerIndex via activePermlinks.indexOf(_containerPermlink) and calls
queryClient.invalidateQueries({ queryKey: [QUERIES.WAVES.GET, host,
_containerPermlink, _containerIndex] }). Also consider adding a short comment
that this is an invalidate-only path to distinguish it from _injectPostCache
which performs optimistic cache writes.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/quickPostModal/usePostSubmitter.ts (1)
55-214:⚠️ Potential issue | 🟡 MinorSurface posting-authority prompt failures to the user.
The inner catch returns false silently, so users get no feedback when the prompt fails. Consider rethrowing to trigger the outer alert (or alert here).Suggested fix
} catch (error) { console.warn('Failed to grant posting authority:', error); - return false; + throw error; } finally { setPostingAuthorityPromptShown(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/quickPostModal/usePostSubmitter.ts` around lines 55 - 214, The inner catch inside the posting-authority prompt flow swallows errors (in the block around shouldPromptPostingAuthority / getPostingAuthorityPromptShown) so users receive no feedback; update that catch (the one that currently logs 'Failed to grant posting authority:') to surface the failure by either calling Alert.alert with a localized message and error details or rethrowing the error so the outer try/catch (which shows intl.formatMessage 'alert.something_wrong') handles it; ensure you still reset setPostingAuthorityPromptShown(false) in the finally block and preserve the manageSubmittingState setIsSubmitting(false) behavior.
🧹 Nitpick comments (2)
src/providers/queries/postQueries/wavesQueries.ts (1)
133-135: Optional: guard against missingparent_author/parent_permlinkIf a partially-written cached comment lands in
commentsCollectionwithoutparent_authororparent_permlink,parentPathbecomes"undefined/undefined"— which fails the lookup silently. A one-line guard makes the intent explicit:🛡️ Proposed defensive guard
const parentPath = `${_cachedComment.parent_author}/${_cachedComment.parent_permlink}`; + if (!_cachedComment.parent_author || !_cachedComment.parent_permlink) { + return; + } const _containerPermlink = wavesIndexCollection.current[parentPath];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/queries/postQueries/wavesQueries.ts` around lines 133 - 135, The code builds parentPath from _cachedComment.parent_author and parent_permlink without checking for presence, so add a guard before constructing parentPath to handle missing values: verify both _cachedComment.parent_author and _cachedComment.parent_permlink are truthy and if not, set _containerPermlink to undefined (or skip the lookup) instead of creating "undefined/undefined"; then only compute parentPath and read wavesIndexCollection.current[parentPath] when both fields are present to avoid silent lookup failures.src/utils/mediaPickerError.ts (1)
9-18: Handle non-Error throws in cancellation detection.
If callers throw strings,error?.messageis empty and cancellation keywords won’t match. Consider falling back toString(error).Suggested tweak
- const message = String(error?.message || '').toLowerCase(); + const message = String(error?.message || error || '').toLowerCase();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/mediaPickerError.ts` around lines 9 - 18, The cancellation detector is missing non-Error throws because message is derived only from error?.message; update isMediaPickerCancellation to fallback to the entire error when message is absent (use error?.message ?? error) so String(...) captures thrown strings or other primitives, then continue using code and message checks (variables: code, message) to test E_PICKER_CANCELLED and 'cancelled'/'canceled'. Ensure you only change the message derivation so other logic remains the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/uploadsGalleryModal/children/speakUploaderModal.tsx`:
- Around line 28-29: The alert is still shown for user-cancelled picker actions
even though reportMediaPickerError already ignores cancellations; in both places
in speakUploaderModal.tsx where reportMediaPickerError is invoked, after calling
reportMediaPickerError check its return (or detect the cancellation from the
thrown error) and immediately return early when it indicates a user cancellation
so the alert is not shown; update both occurrences that call
reportMediaPickerError to bail out before calling alert or showing UI noise.
In `@src/components/uploadsGalleryModal/container/uploadsGalleryModal.tsx`:
- Line 18: The code currently calls reportMediaPickerError and constructs/shows
an error dialog even when the user cancelled the media picker; update each error
handler that calls reportMediaPickerError or builds the dialog (references:
reportMediaPickerError) to first check isMediaPickerCancellation(error) and
return early if true. Concretely, add a guard like "if
(isMediaPickerCancellation(err)) return;" before any reportMediaPickerError(...)
call or before assembling/showing the dialog so cancellation is short‑circuited
(apply the same change to the other occurrences noted in the diff).
In `@src/providers/queries/postQueries/wavesQueries.ts`:
- Around line 127-145: _invalidateWaveContainerForComment currently assumes
_cachedComment.parent_permlink is a top-level wave and uses
wavesIndexCollection.current[parentPath], which fails for reply-to-reply; update
the function to walk the comment ancestry via cache.commentsCollection (starting
from _cachedComment.parent_author/_cachedComment.parent_permlink) in a loop
until you find a parentPath present in wavesIndexCollection.current (or exhaust
the chain), then use that found _containerPermlink and its index in
activePermlinks to call queryClient.invalidateQueries([QUERIES.WAVES.GET, host,
_containerPermlink, _containerIndex]); this ensures nested replies trigger the
same invalidation logic as top-level replies.
---
Outside diff comments:
In `@src/components/quickPostModal/usePostSubmitter.ts`:
- Around line 55-214: The inner catch inside the posting-authority prompt flow
swallows errors (in the block around shouldPromptPostingAuthority /
getPostingAuthorityPromptShown) so users receive no feedback; update that catch
(the one that currently logs 'Failed to grant posting authority:') to surface
the failure by either calling Alert.alert with a localized message and error
details or rethrowing the error so the outer try/catch (which shows
intl.formatMessage 'alert.something_wrong') handles it; ensure you still reset
setPostingAuthorityPromptShown(false) in the finally block and preserve the
manageSubmittingState setIsSubmitting(false) behavior.
---
Nitpick comments:
In `@src/providers/queries/postQueries/wavesQueries.ts`:
- Around line 133-135: The code builds parentPath from
_cachedComment.parent_author and parent_permlink without checking for presence,
so add a guard before constructing parentPath to handle missing values: verify
both _cachedComment.parent_author and _cachedComment.parent_permlink are truthy
and if not, set _containerPermlink to undefined (or skip the lookup) instead of
creating "undefined/undefined"; then only compute parentPath and read
wavesIndexCollection.current[parentPath] when both fields are present to avoid
silent lookup failures.
In `@src/utils/mediaPickerError.ts`:
- Around line 9-18: The cancellation detector is missing non-Error throws
because message is derived only from error?.message; update
isMediaPickerCancellation to fallback to the entire error when message is absent
(use error?.message ?? error) so String(...) captures thrown strings or other
primitives, then continue using code and message checks (variables: code,
message) to test E_PICKER_CANCELLED and 'cancelled'/'canceled'. Ensure you only
change the message derivation so other logic remains the same.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/quickPostModal/usePostSubmitter.ts (1)
242-244:⚠️ Potential issue | 🟡 MinorUnguarded
err.messageaccess could throw iferris not anErrorobject.This is pre-existing code, but now that
_submitReplyhas a robust outer catch that always returnsfalse(never throws), this catch block would only fire for errors fromfetchLatestWavesContainer. If that throws a non-Errorvalue (e.g., a string orundefined),err.messagewill beundefined— which is benign forAlert.alert— but iferritself isnull/undefined, this line will throw an unhandled NPE.Consider a defensive access:
Suggested fix
- Alert.alert('Fail', err.message); + Alert.alert('Fail', err?.message || String(err));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/quickPostModal/usePostSubmitter.ts` around lines 242 - 244, The catch block that calls Alert.alert('Fail', err.message) can throw if err is null/undefined or not an Error; update the catch to defensively derive a message (e.g., if err is an object with a message use that, else if err is a string use it, else fall back to String(err) or a literal like 'Unknown error') and pass that computed message to Alert.alert in place of err.message while keeping the same return false behavior; locate the catch where Alert.alert is called in usePostSubmitter.ts (the catch surrounding fetchLatestWavesContainer/_submitReply) and replace the direct err.message access with the guarded message extraction.
🧹 Nitpick comments (1)
src/utils/mediaPickerError.ts (1)
9-19: Broad substring match onmessagecould suppress non-cancellation errors.The
message.includes('cancelled') || message.includes('canceled')checks are quite broad. If a genuine error happens to contain those substrings (e.g.,"Upload was canceled by the server due to invalid format"), it would be silently swallowed. Consider tightening the match — for instance, checking for'user cancelled'or anchoring to the known library messages — or at minimum, checking the code first and only falling back to the message match whencodeis empty.That said, in the narrow context of
react-native-image-crop-picker, this is unlikely to cause problems today.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/mediaPickerError.ts` around lines 9 - 19, The isMediaPickerCancellation function currently treats any message containing "cancelled"/"canceled" as a cancellation which can hide real errors; update isMediaPickerCancellation to prefer checking the normalized code first (code === 'E_PICKER_CANCELLED' || code === 'E_PICKER_CANCELLED_KEY') and only perform a message-based fallback when code is empty/undefined, and tighten the message check to specific, less-broad patterns (e.g., exact known library messages or regex with word boundaries like /\b(user )?cancel(?:led|ed)\b/ or match startsWith/equals for known phrases) so unrelated messages are not misclassified. Ensure changes are applied within the isMediaPickerCancellation function and preserve its return boolean behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/quickPostModal/usePostSubmitter.ts`:
- Around line 242-244: The catch block that calls Alert.alert('Fail',
err.message) can throw if err is null/undefined or not an Error; update the
catch to defensively derive a message (e.g., if err is an object with a message
use that, else if err is a string use it, else fall back to String(err) or a
literal like 'Unknown error') and pass that computed message to Alert.alert in
place of err.message while keeping the same return false behavior; locate the
catch where Alert.alert is called in usePostSubmitter.ts (the catch surrounding
fetchLatestWavesContainer/_submitReply) and replace the direct err.message
access with the guarded message extraction.
---
Nitpick comments:
In `@src/utils/mediaPickerError.ts`:
- Around line 9-19: The isMediaPickerCancellation function currently treats any
message containing "cancelled"/"canceled" as a cancellation which can hide real
errors; update isMediaPickerCancellation to prefer checking the normalized code
first (code === 'E_PICKER_CANCELLED' || code === 'E_PICKER_CANCELLED_KEY') and
only perform a message-based fallback when code is empty/undefined, and tighten
the message check to specific, less-broad patterns (e.g., exact known library
messages or regex with word boundaries like /\b(user )?cancel(?:led|ed)\b/ or
match startsWith/equals for known phrases) so unrelated messages are not
misclassified. Ensure changes are applied within the isMediaPickerCancellation
function and preserve its return boolean behavior.
This was referenced Feb 19, 2026
This was referenced Mar 3, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Bug Fixes
Improvements
Localization