From a0dc9ddad00b3170092f68c3e329b30fd3e93b6f Mon Sep 17 00:00:00 2001
From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com>
Date: Sun, 14 Dec 2025 10:21:43 +0000
Subject: [PATCH] Address code review suggestions

- Update README.md example to match new OfferFactory signature
- Add error handling and RTCPeerConnection cleanup on factory failure
- Document setupIceCandidateHandler() method usage
- Use undefined instead of null for offerId variable

Co-authored-by: Bas <bvdaakster@users.noreply.github.com>
---
 README.md      |  6 +++---
 src/rondevu.ts | 18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index c0bd52b..f6561b0 100644
--- a/README.md
+++ b/README.md
@@ -49,8 +49,8 @@ const rondevu = await Rondevu.connect({
 await rondevu.publishService({
   service: 'chat:1.0.0',
   maxOffers: 5,  // Maintain up to 5 concurrent offers
-  offerFactory: async (rtcConfig) => {
-    const pc = new RTCPeerConnection(rtcConfig)
+  offerFactory: async (pc) => {
+    // pc is created by Rondevu with ICE handlers already attached
     const dc = pc.createDataChannel('chat')
 
     dc.addEventListener('open', () => {
@@ -64,7 +64,7 @@ await rondevu.publishService({
 
     const offer = await pc.createOffer()
     await pc.setLocalDescription(offer)
-    return { pc, dc, offer }
+    return { dc, offer }
   }
 })
 
diff --git a/src/rondevu.ts b/src/rondevu.ts
index 90ce68b..a6b9a0a 100644
--- a/src/rondevu.ts
+++ b/src/rondevu.ts
@@ -382,6 +382,10 @@ export class Rondevu {
 
     /**
      * Set up ICE candidate handler to send candidates to the server
+     *
+     * Note: This is used by connectToService() where the offerId is already known.
+     * For createOffer(), we use inline ICE handling with early candidate queuing
+     * since the offerId isn't available until after the factory completes.
      */
     private setupIceCandidateHandler(
         pc: RTCPeerConnection,
@@ -434,7 +438,7 @@ export class Rondevu {
         // This ensures we capture all candidates, even those generated immediately
         // when setLocalDescription() is called in the factory
         const earlyIceCandidates: RTCIceCandidateInit[] = []
-        let offerId: string | null = null
+        let offerId: string | undefined
 
         pc.onicecandidate = async (event) => {
             if (event.candidate) {
@@ -461,7 +465,17 @@ export class Rondevu {
         // 3. Call the factory with the pc - factory creates data channel and offer
         // When factory calls setLocalDescription(), ICE gathering starts and
         // candidates are captured by the handler we set up above
-        const { dc, offer } = await this.offerFactory(pc)
+        let dc: RTCDataChannel | undefined
+        let offer: RTCSessionDescriptionInit
+        try {
+            const factoryResult = await this.offerFactory(pc)
+            dc = factoryResult.dc
+            offer = factoryResult.offer
+        } catch (err) {
+            // Clean up the connection if factory fails
+            pc.close()
+            throw err
+        }
 
         // 4. Publish to server to get offerId
         const result = await this.api.publishService({