fix: pass ErrorInfo status code before Ably code in client-side errors - #1248
RaphaelFakhri wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (14)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe changes correct error code and HTTP status code assignments in Android, HTTP, authentication, realtime, transport, and error conversion paths. Tests check the values reported by ping and host validation errors. ChangesErrorInfo assignments
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The corrected error values are ready to merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The correction changes the error codes applications receive, but the examined authentication, connection, and push-registration paths retain their existing acceptance and failure behavior. No introduced security issue was established. Application-specific handling of the corrected values remains unknown. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. A rabbit checks the codes in place, Comment |
Fixes #793
Description
ErrorInfo(String message, int statusCode, int code)takes the HTTP status code before the Ably error code. Several client-side errors passed the arguments the other way round, for examplenew ErrorInfo("...", 40000, 400). The resulting error reportedcode = 400andstatusCode = 40000.This change swaps the arguments so these errors report
code = 40000andstatusCode = 400(andcode = 50000,statusCode = 500for the two server-style errors). The affected errors are:ConnectionManager.pingwhen the connection is notconnected, and the heartbeat timeoutHostsoption validation (fallbackHosts,fallbackHostsUseDefault,environment)HttpCoreproxy configuration,HttpAuthheader parsingAuthnull key,AblyRealtime.channels.getreattach options,ChannelBaseuntilAttachvalidationErrorInfo.fromThrowablePlatform,Push,ActivationContext,ActivationStateMachineTesting
HostsTest.hosts_invalid_options_error_codesassertscode40000 andstatusCode400 for invalid host options.ConnectionManagerPingErrorTestasserts the same forpingon a client that is not connected.Run them with:
Summary by CodeRabbit