feat: can accept base64 payload while proxying to provider#2201
feat: can accept base64 payload while proxying to provider#2201stalniy merged 3 commits intoakash-network:mainfrom
Conversation
WalkthroughThe PR encodes frontend shell WebSocket payloads as base64 and adds Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DeployWeb as Deploy Web (frontend)
participant WS as WebSocket
participant ProviderProxy as Provider Proxy (backend)
participant Provider
Client->>DeployWeb: send shell bytes
DeployWeb->>DeployWeb: bytes → binary string → base64\nattach isBase64: true
DeployWeb->>WS: send encoded payload (isBase64: true)
WS->>ProviderProxy: deliver message
rect rgb(240,245,255)
Note over ProviderProxy: inspect message.isBase64
alt isBase64 == true
ProviderProxy->>ProviderProxy: decode base64 → bytes
else isBase64 == false
ProviderProxy->>ProviderProxy: legacy parse (split/charCode → bytes)
end
end
ProviderProxy->>Provider: proxy reconstructed payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
🧬 Code graph analysis (1)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2201 +/- ##
==========================================
- Coverage 47.24% 46.90% -0.34%
==========================================
Files 1023 1013 -10
Lines 29105 28758 -347
Branches 7587 7562 -25
==========================================
- Hits 13750 13489 -261
+ Misses 15041 14886 -155
- Partials 314 383 +69
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts(1 hunks)apps/provider-proxy/src/services/WebsocketServer.ts(1 hunks)apps/provider-proxy/src/utils/schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.tsapps/provider-proxy/src/utils/schema.tsapps/provider-proxy/src/services/WebsocketServer.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.tsapps/provider-proxy/src/utils/schema.tsapps/provider-proxy/src/services/WebsocketServer.ts
🧬 Code graph analysis (2)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
apps/indexer/drizzle/schema.ts (1)
message(530-576)
apps/provider-proxy/src/services/WebsocketServer.ts (1)
apps/indexer/drizzle/schema.ts (1)
message(530-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (2)
apps/provider-proxy/src/utils/schema.ts (1)
32-36: Temporary field is appropriately designed for gradual migration.The optional
isBase64field with a default offalseprovides good backward compatibility during the transition period. Since the PR description mentions this will be removed in a follow-up change, consider adding a TODO comment or tracking issue reference to ensure the cleanup happens.apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
306-307: LGTM! Base64 flag is correctly set for shell connections.The
isBase64: trueflag properly indicates to the proxy that the payload will be base64-encoded, enabling the new encoding path while maintaining backward compatibility.
425b8cd to
d62c950
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
311-312: Consider a loop-based approach for better performance on large payloads.The current implementation correctly avoids the stack overflow issue from the previous spread operator approach. However, string concatenation in
reducecreates intermediate string objects on each iteration, which can be slow for large payloads.For better performance with large messages, consider using a traditional loop:
if (message.length > 0) { - const binaryString = Array.from(message).reduce((acc, byte) => acc + String.fromCharCode(byte), ""); - remoteMessage.data = btoa(binaryString); + let binaryString = ''; + for (let i = 0; i < message.length; i++) { + binaryString += String.fromCharCode(message[i]); + } + remoteMessage.data = btoa(binaryString); }Note: This is a micro-optimization that only matters for very large shell payloads (10MB+).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts(2 hunks)apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts(1 hunks)apps/provider-proxy/src/services/WebsocketServer.ts(1 hunks)apps/provider-proxy/src/utils/schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/provider-proxy/src/services/WebsocketServer.ts
- apps/provider-proxy/src/utils/schema.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts
🧠 Learnings (1)
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts
🧬 Code graph analysis (1)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
apps/indexer/drizzle/schema.ts (1)
message(530-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (3)
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts (1)
306-307: LGTM! isBase64 flag added to coordinate base64 payload handling.The addition of
isBase64: truecorrectly signals to the provider-proxy backend that the payload data is base64-encoded, enabling the conditional decoding logic.apps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.ts (2)
945-945: LGTM! Test expectation correctly updated to base64-encoded value.The test now expects the base64-encoded value
"ZWNobyBoZWxsbw=="which correctly represents the encoding of"echo hello", matching the new base64 payload handling in the production code.
1018-1019: LGTM! Test expectations correctly updated to base64-encoded values.The tests now expect the base64-encoded values
"Zmlyc3Q="and"c2Vjb25k"which correctly represent the encodings of"first"and"second", validating that multiple messages are properly base64-encoded.
d62c950 to
d273898
Compare
apps/deploy-web/src/services/provider-proxy/provider-proxy.service.ts
Outdated
Show resolved
Hide resolved
d273898 to
d54dc64
Compare
refs #2178
A follow-up change in a week or so should remove the new field and old version of decoding.
Summary by CodeRabbit
Improvements
Tests