[ECO-4841] Add README section to explain "Connection limit exceeded" error and solutions #1905
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe changes introduced in this pull request enhance the documentation for the Ably JavaScript client library, specifically targeting the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
staging/pull/1905/bundle-report
github-actions
bot
temporarily deployed
to
staging/pull/1905/features
github-actions
bot
temporarily deployed
to
staging/pull/1905/typedoc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
docs/react.md (4)
39-39: Enhance connection management guidance with examplesWhile the guidance about creating the Ably.Realtime instance outside components is correct, consider adding concrete examples to illustrate:
- ❌ Incorrect implementation (inside component)
- ✅ Correct implementation (outside component)
This visual comparison would make it easier for developers to understand and avoid the connection limit issue.
Example addition:
Also, ensure that the `Ably.Realtime` instance is created outside of components to prevent it from being recreated on component re-renders. This will help avoid opening extra unnecessary connections to the Ably servers and potentially reaching the maximum connections limit on your account. + +```jsx +// ❌ Incorrect: Creates new connection on every render +function BadComponent() { + const client = new Ably.Realtime({ key: 'api-key' }); // Don't do this! + return <div>...</div>; +} + +// ✅ Correct: Single connection instance +const client = new Ably.Realtime({ key: 'api-key' }); +function GoodComponent() { + return <div>...</div>; +} +```
Line range hint
391-417: Add TypeScript types and common error scenariosThe error handling section is comprehensive but could be enhanced with:
- TypeScript type definitions for error objects
- Common error scenarios and their solutions
Consider adding:
const { connectionError, channelError } = useChannel('my_channel', messageHandler); + +// TypeScript type hints +type ConnectionError = Ably.ErrorInfo & { + // Add specific connection error properties +}; + +type ChannelError = Ably.ErrorInfo & { + // Add specific channel error properties +}; + +// Common error scenarios +/** + * Common Connection Errors: + * - 40142: Connection limit exceeded + * Solution: Ensure Ably.Realtime instance is created outside components + * - 40171: Invalid API key + * Solution: Check API key format and permissions + * + * Common Channel Errors: + * - 40160: Channel operation not permitted + * Solution: Verify channel permissions in API key capabilities + */
Line range hint
418-450: Add practical examples for state listener hooksThe documentation for new hooks would benefit from real-world examples showing:
- Integration with error handling
- Common use cases like displaying connection status
Consider adding:
useConnectionStateListener((stateChange) => { console.log(stateChange.current); console.log(stateChange.previous); console.log(stateChange.reason); }); + +// Real-world example: Connection status indicator +const ConnectionStatus = () => { + const [status, setStatus] = useState('connected'); + const [error, setError] = useState(null); + + useConnectionStateListener((stateChange) => { + setStatus(stateChange.current); + if (stateChange.reason) { + setError(stateChange.reason.message); + } + }); + + return ( + <div> + <span className={`status-${status}`}> + {status} + </span> + {error && <p className="error">{error}</p>} + </div> + ); +};
Line range hint
543-583: Add version compatibility and related documentation linksConsider enhancing the NextJS warnings section with:
- NextJS version compatibility information
- Links to related NextJS documentation about webpack configuration
Add version information and links:
## NextJS warnings + +> Tested with NextJS versions 12.x and 13.x +> For more information about NextJS webpack configuration, see the [official documentation](https://nextjs.org/docs/api-reference/next.config.js/custom-webpack-config)README.md (2)
648-657: Consider adding TypeScript type annotations to the code example.The explanation and solution are excellent. To make it even more helpful, consider adding TypeScript types to demonstrate type-safe usage:
-const client = new Ably.Realtime({ key: 'your-ably-api-key', autoConnect: typeof window !== 'undefined' }); +const client: Ably.Realtime = new Ably.Realtime({ + key: 'your-ably-api-key', + autoConnect: typeof window !== 'undefined' +});🧰 Tools
🪛 LanguageTool
[uncategorized] ~648-~648: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...'re developing your app, due to Next.js server side rendering your components. Note that ev...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~656-~656: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...e server side, while not affecting your client side components. - If you're using any Reac...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
652-652: Expected: indented; Actual: fenced
Code block style(MD046, code-block-style)
660-663: Consider adding a code example for the recommended file structure.The explanation of the HMR issue and solution is excellent. To make it even more helpful, consider adding a code example showing the recommended file structure:
// ably-client.ts import * as Ably from 'ably'; export const ablyClient = new Ably.Realtime({ key: 'your-ably-api-key', autoConnect: typeof window !== 'undefined' }); // your-component.tsx import { ablyClient } from './ably-client'; // Use ablyClient in your component🧰 Tools
🪛 LanguageTool
[uncategorized] ~662-~662: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ime()call to a separate file, such asably-client.js`, and export the client instance fro...(HYPHENATED_LY_ADVERB_ADJECTIVE)
[style] ~662-~662: Consider shortening or rephrasing this to strengthen your wording.
Context: ...only be recreated when you specifically make changes to theably-client.jsfile, which should...(MAKE_CHANGES)
[uncategorized] ~662-~662: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...en you specifically make changes to theably-client.jsfile, which should be far less freq...(HYPHENATED_LY_ADVERB_ADJECTIVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- README.md (1 hunks)
- docs/react.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~648-~648: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...'re developing your app, due to Next.js server side rendering your components. Note that ev...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~656-~656: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...e server side, while not affecting your client side components. - If you're using any Reac...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~658-~658: This phrase is redundant. Consider using “outside”.
Context: ...nstantiation call (new Ably.Realtime) outside of the components. You can find an example...(OUTSIDE_OF)
[uncategorized] ~662-~662: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ime()call to a separate file, such asably-client.js`, and export the client instance fro...(HYPHENATED_LY_ADVERB_ADJECTIVE)
[style] ~662-~662: Consider shortening or rephrasing this to strengthen your wording.
Context: ...only be recreated when you specifically make changes to theably-client.jsfile, which should...(MAKE_CHANGES)
[uncategorized] ~662-~662: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...en you specifically make changes to theably-client.jsfile, which should be far less freq...(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 Markdownlint
README.md
652-652: Expected: indented; Actual: fenced
Code block style(MD046, code-block-style)
🔇 Additional comments (3)
docs/react.md (1)
Line range hint
451-542: LGTM! Comprehensive documentation of the skip parameterThe documentation effectively covers:
- Purpose and behavior of the skip parameter
- Real-world use cases with examples
- Integration with authentication and premium features
README.md (2)
644-647: LGTM! Clear and informative introduction.The section header and introduction effectively set the context for the common "Connection limit exceeded" error.
658-659: LGTM! Clear explanation with helpful reference.The explanation effectively communicates the React-specific issue and points users to detailed examples in the React documentation.
🧰 Tools
🪛 LanguageTool
[style] ~658-~658: This phrase is redundant. Consider using “outside”.
Context: ...nstantiation call (new Ably.Realtime) outside of the components. You can find an example...(OUTSIDE_OF)
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
VeskeR
force-pushed
the
ECO-4841/document-max-connections-issue-solutions
branch
from
af29f55 to
2d3d1e8
Compare
github-actions
bot
temporarily deployed
to
staging/pull/1905/bundle-report
github-actions
bot
temporarily deployed
to
staging/pull/1905/features
github-actions
bot
temporarily deployed
to
staging/pull/1905/typedoc
…olutions Resolves ECO-4841
VeskeR
force-pushed
the
ECO-4841/document-max-connections-issue-solutions
branch
from
2d3d1e8 to
6c59ffc
Compare
Resolves ECO-4841
This is based on the "max connections errors" research done in ECO-4841 (see comments), "Ably-js max connections issue overview and possible solutions", and solutions found in ably-labs/react-hooks#8 (comment) and #1742 (comment)
Related: #1779, #1742
Summary by CodeRabbit
New Features
useConnectionStateListener,useChannelStateListener, anduseAbly.skipadded touseChannel,usePresence, andusePresenceListenerhooks for better control over attachment behavior.Documentation
ably-js React Hooks.