Conversation
|
Warning Rate limit exceeded@9cb14c1ec0 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 42 seconds before requesting another review. ⌛ 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. 📥 CommitsReviewing files that changed from the base of the PR and between 6a6a1bf2592de75223b245c3981d1ded7d589094 and 46a4efc. 📒 Files selected for processing (1)
WalkthroughReplaces cwd-based script loading with a dirname helper in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Page
participant Auth as AuthController
participant QRGen as generateASCIIQR
participant Host as HostLayer
participant Runtime
Note over Page,Auth: Page detects login canvas / QR available
Page->>Auth: provide QR code string
Auth->>QRGen: generateASCIIQR(code, {small:true})
QRGen-->>Auth: asciiQR or throws
alt asciiQR produced
Auth->>Host: emit/display asciiQR
Host->>Runtime: console.log("waitForQrCodeScan: new QR code detected")
Host->>Runtime: console.log("waitForQrCodeScan: new QR code: create ASCII")
Host->>Runtime: console.log("qr code: ...")
else error
Auth->>Runtime: console.log("asciiQr error / fallback to empty")
end
Host->>Host: continue waiting loop (logged attempts)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between c356dbc and 1a85d08eb34abf8e30221cf6bf7823a9e6853733.
📒 Files selected for processing (1)
src/api/whatsapp.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-08T17:49:03.345Z
Learnt from: CR
PR: venomlib/venom#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:49:03.345Z
Learning: Applies to src/lib/wapi/**/*.js : WAPI layer code is built with webpack and injected into the Puppeteer browser context
Applied to files:
src/api/whatsapp.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/api/whatsapp.ts (1)
78-82: Verify the middleware path assumption.Similar to the WAPI path issue above, ensure that
../../lib/middleware/middleware.jsresolves correctly from the built output location.
🧹 Nitpick comments (1)
src/api/whatsapp.ts (1)
13-20: The getDirectory() helper has portability concerns.While this helper attempts to handle both CommonJS and ESM environments, the approach still has fragility:
- The
@ts-ignorecomments indicate TypeScript is unable to properly type-check this code, which may hide issues.- The code assumes
__dirnameorimport.meta.urlalways resolve correctly, but this depends on the build configuration and module system.Consider these improvements:
Option 1 (Recommended): Add explicit error handling and type guards:
function getDirectory() { + if (typeof __dirname !== 'undefined') { - // @ts-ignore - return typeof __dirname !== 'undefined' - ? // @ts-ignore - __dirname - : // @ts-ignore - dirname(fileURLToPath(import.meta.url)); + return __dirname; + } + if (typeof import.meta !== 'undefined' && import.meta.url) { + return dirname(fileURLToPath(import.meta.url)); + } + throw new Error('Unable to determine directory: neither __dirname nor import.meta.url is available'); }Option 2: Use Node's module resolution as suggested in previous reviews (see past comments).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 1a85d08eb34abf8e30221cf6bf7823a9e6853733 and fcbf7324ffed68337f7ea3e3047224f4a51d212c.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(1 hunks)src/api/layers/host.layer.ts(1 hunks)src/api/whatsapp.ts(2 hunks)src/controllers/auth.ts(3 hunks)src/controllers/initializer.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/api/layers/*.layer.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep functionality layers in src/api/layers using the *.layer.ts naming (e.g., sender.layer.ts, listener.layer.ts, group.layer.ts, profile.layer.ts, controls.layer.ts, retriever.layer.ts)
Files:
src/api/layers/host.layer.ts
src/controllers/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Controllers (initializer.ts for session creation, browser.ts for Puppeteer management, auth.ts for authentication/QR) live under src/controllers
Files:
src/controllers/initializer.tssrc/controllers/auth.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: venomlib/venom#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:49:03.345Z
Learning: Applies to src/lib/wapi/**/*.js : WAPI layer code is built with webpack and injected into the Puppeteer browser context
📚 Learning: 2025-09-08T17:49:03.345Z
Learnt from: CR
PR: venomlib/venom#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:49:03.345Z
Learning: Applies to src/lib/wapi/**/*.js : WAPI layer code is built with webpack and injected into the Puppeteer browser context
Applied to files:
src/api/whatsapp.ts
🧬 Code graph analysis (3)
src/api/layers/host.layer.ts (1)
src/controllers/auth.ts (2)
needsToScan(67-73)asciiQr(93-102)
src/api/whatsapp.ts (1)
scripts/create-wrappers.js (1)
__dirname(8-8)
src/controllers/initializer.ts (3)
app.js (2)
page(24-24)browser(25-25)src/controllers/browser.ts (1)
initWhatsapp(41-122)src/api/whatsapp.ts (1)
🪛 GitHub Actions: Build
src/controllers/auth.ts
[error] 3-3: Cannot find module '../utils/qr-generator.js' or its corresponding type declarations.
🪛 GitHub Check: build
src/controllers/auth.ts
[failure] 3-3:
Cannot find module '../utils/qr-generator.js' or its corresponding type declarations.
🔇 Additional comments (8)
package.json (1)
28-30: LGTM! Build pipeline enhancement for vendor assets.The addition of
copy:vendorto the build pipeline and the new script to copy vendor assets to both CommonJS and ESM distributions aligns well with the PR's objective of fixing script loading. Usingshxensures cross-platform compatibility.src/controllers/auth.ts (2)
93-102: Good error handling in asciiQr.The refactored
asciiQrfunction properly handles errors by logging them and returning an empty string on failure, which prevents silent failures. The async implementation is appropriate for callinggenerateASCIIQR.
16-16: Simplified QR detection logic is sufficient. No occurrences ofelLoginWrapper3orelLoginWrapper4were found in the codebase.src/api/layers/host.layer.ts (1)
147-182: LGTM! Enhanced observability for QR code scanning.The added console.log statements provide clear visibility into the QR code scanning workflow without altering the underlying logic. This aligns well with the broader PR changes that shift from spinnies-based progress updates to console logging.
src/controllers/initializer.ts (3)
254-258: Good: Added explicit logging around WhatsApp initialization.The console.log statements around
initWhatsappprovide clear visibility into the initialization flow, which will help with debugging when script loading issues occur.
269-272: LGTM! Improved logging for initialization stages.These console.log additions provide clear checkpoints during the initialization process, replacing some of the spinnies-based status updates. This improves debuggability.
337-347: Consistent logging for QR code interface states.The replacement of spinnies-based messages with console.log for QR code states (opening, loading, loaded) is consistent with the broader shift to console-based logging throughout the PR. This maintains observability while simplifying the status display mechanism.
src/api/whatsapp.ts (1)
70-74: No action required – webpack outputs wapi.js to dist/lib/wapi, so from dist/{cjs,esm}/api ../../lib/wapi/wapi.js resolves correctly.
This reverts commit e44bc44.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/qr-generator.ts (1)
25-31: Load each vendor module once and reuse its namespace.We call
import('../../vendor/QRCode/index.js')twice to fall back from.default, which repeats the dynamic load. Grabbing the module namespace once and reusing it trims the extra import while keeping compatibility.- const QRCode = - (await import('../../vendor/QRCode/index.js')).default || - (await import('../../vendor/QRCode/index.js')); - const QRErrorCorrectLevel = - (await import('../../vendor/QRCode/QRErrorCorrectLevel.js')).default || - (await import('../../vendor/QRCode/QRErrorCorrectLevel.js')); + const qrModule = await import('../../vendor/QRCode/index.js'); + const QRCode = qrModule.default ?? qrModule; + const levelModule = await import('../../vendor/QRCode/QRErrorCorrectLevel.js'); + const QRErrorCorrectLevel = levelModule.default ?? levelModule;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between fcbf7324ffed68337f7ea3e3047224f4a51d212c and 1273f8e1adde5bf33185fa3903121839fa9ffa8f.
📒 Files selected for processing (12)
package.json(1 hunks)src/utils/qr-generator.ts(1 hunks)vendor/QRCode/QR8bitByte.js(1 hunks)vendor/QRCode/QRBitBuffer.js(1 hunks)vendor/QRCode/QRErrorCorrectLevel.js(1 hunks)vendor/QRCode/QRMaskPattern.js(1 hunks)vendor/QRCode/QRMath.js(1 hunks)vendor/QRCode/QRMode.js(1 hunks)vendor/QRCode/QRPolynomial.js(1 hunks)vendor/QRCode/QRRSBlock.js(1 hunks)vendor/QRCode/QRUtil.js(1 hunks)vendor/QRCode/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 Biome (2.1.2)
vendor/QRCode/QRMath.js
[error] 30-30: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 37-37: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
# Conflicts: # .eslintrc.cjs # eslint.config.js # package-lock.json # package.json # src/api/helpers/callback-wile.ts # src/controllers/check-up-to-date.ts # src/controllers/initializer.ts # src/lib/counter/gulpfile.mjs
This reverts commit e44bc44.
1 participant
Fixes # .
Changes proposed in this pull request
To test (it takes a while):
npm install github:<username>/venom#<branch>Summary by CodeRabbit