ci: smoke tests using wwebjs#222
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive WhatsApp smoke testing framework, adding new TypeScript modules for session persistence, status reporting, and test orchestration. It updates dependencies and CI/CD workflows to execute smoke tests on a schedule and via manual triggers, validating chat flows end-to-end. ChangesWhatsApp Smoke Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR introduces a new, self-contained WhatsApp smoke test framework with infrastructure services, orchestration logic, and CI/CD integration. Complexity stems from multi-component interactions (WhatsApp client, GCS, Instatus API), async/await patterns, and dependency management. Changes are cohesive and follow consistent patterns, but require understanding of RemoteAuth flows, message collection logic, and environment variable wiring. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 8
🧹 Nitpick comments (2)
.github/workflows/smoke.yml (1)
30-30: ⚖️ Poor tradeoffConsider pinning actions to SHA hashes.
Using action tags (e.g.,
@v4) instead of commit SHAs can expose the workflow to supply chain attacks if the tag is moved or the action is compromised. Pinning to immutable SHA hashes provides stronger security guarantees.🔐 Example with SHA pinning
- - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0Note: You can find current SHA hashes for these actions in their respective GitHub repositories.
Also applies to: 32-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/smoke.yml at line 30, The workflow uses mutable action tags (e.g., "actions/checkout@v4") which can be moved and pose supply-chain risk; replace those tag references with the corresponding immutable commit SHAs for actions/checkout and the other action referenced (line 32) by updating the uses entries to the full "actions/checkout@<SHA>" form (and the equivalent SHA for the other action) so the workflow pins to a specific commit instead of a floating tag.cypress/e2e/smoke.spec.ts (1)
54-55: ⚡ Quick winConsider a more dynamic wait strategy.
The hardcoded 30-second wait may be unnecessary if messages arrive faster, or insufficient if they're delayed. Consider polling for a specific message count or using a custom wait condition that checks for message presence with a reasonable timeout.
💡 Example using should assertion with retry
- // Wait for 30 seconds to ensure all messages are in - cy.wait(30000); - - cy.get('[data-testid="messageContainer"] [data-testid="content"]', { timeout: 10000 }).then( + // Wait for at least 3 messages with auto-retry + cy.get('[data-testid="messageContainer"] [data-testid="content"]', { timeout: 30000 }) + .should('have.length.at.least', 3) + .then( ($messages) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cypress/e2e/smoke.spec.ts` around lines 54 - 55, Replace the hardcoded cy.wait(30000) with a dynamic polling/assertion that waits for the expected messages instead of a fixed timeout: stop using the cy.wait(30000) statement and instead use Cypress retryable assertions (e.g., cy.get('<message-list-selector>').should('have.length.at.least', EXPECTED_COUNT, { timeout: 30000 }) or cy.contains('<expected-message-text>').should('exist', { timeout: 30000 })) so the test proceeds as soon as messages arrive but still fails after a reasonable timeout; update the selector/text and EXPECTED_COUNT to match the application’s message container used in this spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/smoke.yml:
- Around line 47-48: The "Write GCP credentials" step writes the service account
JSON to /tmp/gcp-key.json with no explicit permissions; update the step so the
file is created with restrictive permissions (owner read/write only) by e.g.
setting a restrictive umask before writing or immediately running chmod 600 on
/tmp/gcp-key.json after the echo, ensuring the credentials file is only readable
by the runner process.
In `@cypress/e2e/smoke.spec.ts`:
- Around line 63-68: The current assertion builds a strict expected string in
dateString and checks Cypress.$(lastThree[0]).text() for an exact match, which
is flaky around midnights/timezones; update the assertion in the test (where
dateString and lastThree are used) to be tolerant: either (a) generate a small
set of allowed date strings (today, yesterday, tomorrow) and assert the WhatsApp
message contains any of those, or (b) replace the exact match with a regex that
validates the DD/MM/YYYY format (e.g., \d{2}\/\d{2}\/\d{4}) and only assert
format, not exact value; apply this change where dateString is defined and where
expect(...).to.contain(...) is called.
- Around line 34-35: Remove the redundant second click on the password field: in
the test where cy.get('[data-testid="outlinedInput"] [name="password"]').click()
is called twice, keep a single cy.get(...).click() before typing into the field
(remove the duplicate call referencing the same selector).
In `@wwebjs/smoke/flows.ts`:
- Around line 44-45: The botPhoneNumber fallback currently hard-codes a real
number; remove the '+918657048982' default and make BOT_PHONE_NUMBER required by
replacing the inline fallback with a strict check (read
process.env.BOT_PHONE_NUMBER into botPhoneNumber and if falsy throw a clear
error during startup), or set botPhoneNumber to undefined and add startup
validation that throws; update any code that uses botPhoneNumber (e.g.,
references in flows.ts) to rely on this validation so the process fails fast
instead of sending to an unintended recipient.
In `@wwebjs/smoke/gcs-store.ts`:
- Around line 73-75: The catch block that handles deletion from GCS currently
checks (err as NodeJS.ErrnoException)?.code !== '404' using a string; change
this to treat the GCS ApiError code as a number (compare to 404) and
additionally tolerate the HTTP response shape by checking err.code !== 404 ||
err.response?.statusCode !== 404 (or explicitly accept either 404 as number or
response.statusCode === 404) so that missing-object errors are swallowed and
other errors are rethrown; update the catch around the bucket.file(...).delete()
call accordingly and avoid relying on a top-level string '404'.
In `@wwebjs/smoke/instatus.ts`:
- Around line 14-23: The function reportToInstatus currently logs success for
any HTTP response and only treats transport errors as failures; update
reportToInstatus to check the fetch Response (res) using res.ok after the PUT
and treat non-2xx responses as failures by logging an error and throwing or
returning a rejected result so callers can handle it; locate the fetch call and
the console.log("Instatus report...") that follows it, replace or augment that
logic to inspect res.ok (and optionally include res.status/res.statusText and
body) and handle non-ok responses as errors.
In `@wwebjs/smoke/runner.ts`:
- Around line 27-38: The waitForRemoteSessionSaved Promise attaches its
'remote_session_saved' listener too late and can miss events that fired earlier;
update waitForRemoteSessionSaved to first check an existing saved-state on the
client (e.g., a boolean property or getter like client.remoteSessionSaved or
client.isRemoteSessionSaved(), add that property if it doesn't exist), resolve
immediately if true, otherwise attach client.once('remote_session_saved', ...)
and start the timeout; also ensure any other usage sites that currently add the
listener in finally follow the same pattern (check saved-state first then
attach) so the event cannot be missed.
- Around line 145-148: The runner currently only validates env.gcsBucket and
exits on missing value; add the same fail-fast checks for env.botPhoneNumber and
env.sessionId so missing bot/session configuration is reported immediately.
Update the validation block (the check around env.gcsBucket in
wwebjs/smoke/runner.ts) to validate env.gcsBucket, env.botPhoneNumber and
env.sessionId, using console.error() with actionable messages for each missing
variable and process.exit(1) to stop startup when any are absent.
---
Nitpick comments:
In @.github/workflows/smoke.yml:
- Line 30: The workflow uses mutable action tags (e.g., "actions/checkout@v4")
which can be moved and pose supply-chain risk; replace those tag references with
the corresponding immutable commit SHAs for actions/checkout and the other
action referenced (line 32) by updating the uses entries to the full
"actions/checkout@<SHA>" form (and the equivalent SHA for the other action) so
the workflow pins to a specific commit instead of a floating tag.
In `@cypress/e2e/smoke.spec.ts`:
- Around line 54-55: Replace the hardcoded cy.wait(30000) with a dynamic
polling/assertion that waits for the expected messages instead of a fixed
timeout: stop using the cy.wait(30000) statement and instead use Cypress
retryable assertions (e.g.,
cy.get('<message-list-selector>').should('have.length.at.least', EXPECTED_COUNT,
{ timeout: 30000 }) or cy.contains('<expected-message-text>').should('exist', {
timeout: 30000 })) so the test proceeds as soon as messages arrive but still
fails after a reasonable timeout; update the selector/text and EXPECTED_COUNT to
match the application’s message container used in this spec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfe5201b-6fc6-418c-99cf-ce76a8089e0f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
.cursorignore.github/workflows/e2e.yml.github/workflows/smoke.yml.gitignorecypress/e2e/smoke.spec.tspackage.jsonwwebjs/smoke/flows.tswwebjs/smoke/gcs-store.tswwebjs/smoke/instatus.tswwebjs/smoke/runner.tswwebjs/smoke/tsconfig.json
| - name: Write GCP credentials | ||
| run: echo '${{ secrets.GCP_SA_KEY_JSON }}' > /tmp/gcp-key.json |
There was a problem hiding this comment.
Set restrictive permissions on the credentials file.
Writing the GCP service account key to a file without explicit permissions could expose it to other processes. While GitHub Actions runners are isolated, it's best practice to restrict file permissions.
🔒 Proposed fix
- name: Write GCP credentials
- run: echo '${{ secrets.GCP_SA_KEY_JSON }}' > /tmp/gcp-key.json
+ run: |
+ echo '${{ secrets.GCP_SA_KEY_JSON }}' > /tmp/gcp-key.json
+ chmod 600 /tmp/gcp-key.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Write GCP credentials | |
| run: echo '${{ secrets.GCP_SA_KEY_JSON }}' > /tmp/gcp-key.json | |
| - name: Write GCP credentials | |
| run: | | |
| echo '${{ secrets.GCP_SA_KEY_JSON }}' > /tmp/gcp-key.json | |
| chmod 600 /tmp/gcp-key.json |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/smoke.yml around lines 47 - 48, The "Write GCP
credentials" step writes the service account JSON to /tmp/gcp-key.json with no
explicit permissions; update the step so the file is created with restrictive
permissions (owner read/write only) by e.g. setting a restrictive umask before
writing or immediately running chmod 600 on /tmp/gcp-key.json after the echo,
ensuring the credentials file is only readable by the runner process.
| cy.get('[data-testid="outlinedInput"] [name="password"]').click(); | ||
| cy.get('[data-testid="outlinedInput"] [name="password"]').click(); |
There was a problem hiding this comment.
Remove redundant click.
The password field is clicked twice before typing. Only one click is needed to focus the field.
🔧 Proposed fix
- cy.get('[data-testid="outlinedInput"] [name="password"]').click();
cy.get('[data-testid="outlinedInput"] [name="password"]').click();
cy.get('[data-testid="outlinedInput"] [name="password"]').type(SMOKE_TEST_LOGIN_PASSWORD);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cypress/e2e/smoke.spec.ts` around lines 34 - 35, Remove the redundant second
click on the password field: in the test where
cy.get('[data-testid="outlinedInput"] [name="password"]').click() is called
twice, keep a single cy.get(...).click() before typing into the field (remove
the duplicate call referencing the same selector).
| const now = new Date(); | ||
| const year = now.getFullYear(); | ||
| const month = String(now.getMonth() + 1).padStart(2, '0'); | ||
| const day = String(now.getDate()).padStart(2, '0'); | ||
| const dateString = `World! ${day}/${month}/${year}`; | ||
| expect(Cypress.$(lastThree[0]).text()).to.contain(dateString); |
There was a problem hiding this comment.
Date assertion may cause flaky failures.
The test constructs the expected date string from new Date() at test execution time and assumes the WhatsApp message contains the same date. This creates timing issues:
- Tests running near midnight may fail if the message timestamp crosses the day boundary
- Timezone differences between the test runner and the WhatsApp bot could cause mismatches
- Message delays could result in different dates
Consider either:
- Accepting a date within a reasonable range (today ± 1 day)
- Using a regex pattern to validate date format rather than exact date
- Making the flow send a known static value instead of a dynamic date
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cypress/e2e/smoke.spec.ts` around lines 63 - 68, The current assertion builds
a strict expected string in dateString and checks Cypress.$(lastThree[0]).text()
for an exact match, which is flaky around midnights/timezones; update the
assertion in the test (where dateString and lastThree are used) to be tolerant:
either (a) generate a small set of allowed date strings (today, yesterday,
tomorrow) and assert the WhatsApp message contains any of those, or (b) replace
the exact match with a regex that validates the DD/MM/YYYY format (e.g.,
\d{2}\/\d{2}\/\d{4}) and only assert format, not exact value; apply this change
where dateString is defined and where expect(...).to.contain(...) is called.
| botPhoneNumber: process.env.BOT_PHONE_NUMBER ?? '+918657048982', | ||
| sessionId: process.env.WHATSAPP_SESSION_ID ?? 'production-sender', |
There was a problem hiding this comment.
Remove the hard-coded production phone fallback.
If BOT_PHONE_NUMBER is not set, the run still targets a real number by default. With the current production default selection, that can send smoke traffic to an unintended recipient instead of failing fast.
Suggested fix
- botPhoneNumber: process.env.BOT_PHONE_NUMBER ?? '+918657048982',
+ botPhoneNumber: process.env.BOT_PHONE_NUMBER ?? '',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| botPhoneNumber: process.env.BOT_PHONE_NUMBER ?? '+918657048982', | |
| sessionId: process.env.WHATSAPP_SESSION_ID ?? 'production-sender', | |
| botPhoneNumber: process.env.BOT_PHONE_NUMBER ?? '', | |
| sessionId: process.env.WHATSAPP_SESSION_ID ?? 'production-sender', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wwebjs/smoke/flows.ts` around lines 44 - 45, The botPhoneNumber fallback
currently hard-codes a real number; remove the '+918657048982' default and make
BOT_PHONE_NUMBER required by replacing the inline fallback with a strict check
(read process.env.BOT_PHONE_NUMBER into botPhoneNumber and if falsy throw a
clear error during startup), or set botPhoneNumber to undefined and add startup
validation that throws; update any code that uses botPhoneNumber (e.g.,
references in flows.ts) to rely on this validation so the process fails fast
instead of sending to an unintended recipient.
| } catch (err: unknown) { | ||
| if ((err as NodeJS.ErrnoException)?.code !== '404') throw err; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the Node.js @google-cloud/storageclient, what fields/types are used on errors thrown byfile.delete() when the object does not exist?
💡 Result:
When you call bucket.file(name).delete() and the object does not exist, the client throws an ApiError (from @google-cloud/common) whose error body includes these fields, which are then exposed as properties on the thrown error: 1) code (number | undefined) - ApiError has a code?: number property. [1] - The thrown error for a missing object includes code: 404 in its body. [2] 2) message (string) - ApiError is constructed with a message string (and thus the thrown error has e.message). [3] - The missing-object case shows a message like No such object: <bucket>/<object>. [4] 3) errors (GoogleInnerError[] | undefined) - ApiError has an errors?: GoogleInnerError[] property. [1] - In the missing-object case, the error body includes errors: [Array]. [2] 4) response (r.Response | undefined) - ApiError has a response?: r.Response property. [1] - This response is set from errorBody.response in @google-cloud/common’s util. [3] Additionally, for the specific “not found” HTTP status: - The HTTP status code is 404, as shown by the missing-object error body having code: 404. [2] These are the fields/types you can rely on on the thrown error instance for this scenario: code?: number, message: string, errors?: GoogleInnerError[], and response?: r.Response. [1][3][2][4]
Citations:
- 1: https://cloud.google.com/nodejs/docs/reference/common/latest/common/apierror
- 2: file.delete returns a wrong value when the files doesn't exist googleapis/nodejs-storage#2182
- 3: https://github.com/googleapis/nodejs-common/blob/main/src/util.ts
- 4: UnhandledPromiseRejection when deleting an object that doesn't exist in a bucket googleapis/nodejs-storage#867
Handle GCS “not found” errors by checking numeric HTTP 404.
bucket.file(...).delete() throws an ApiError where the missing-object case sets code: 404 as a number; comparing to '404' can incorrectly rethrow. Update the catch to test err.code !== 404 (optionally fall back to err.response?.statusCode), and don’t rely on statusCode being a top-level field.
Suggested fix
- } catch (err: unknown) {
- if ((err as NodeJS.ErrnoException)?.code !== '404') throw err;
+ } catch (err: unknown) {
+ const e = err as { code?: number; response?: { statusCode?: number } };
+ if (e.code !== 404 && e.response?.statusCode !== 404) throw err;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err: unknown) { | |
| if ((err as NodeJS.ErrnoException)?.code !== '404') throw err; | |
| } | |
| } catch (err: unknown) { | |
| const e = err as { code?: number; response?: { statusCode?: number } }; | |
| if (e.code !== 404 && e.response?.statusCode !== 404) throw err; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wwebjs/smoke/gcs-store.ts` around lines 73 - 75, The catch block that handles
deletion from GCS currently checks (err as NodeJS.ErrnoException)?.code !==
'404' using a string; change this to treat the GCS ApiError code as a number
(compare to 404) and additionally tolerate the HTTP response shape by checking
err.code !== 404 || err.response?.statusCode !== 404 (or explicitly accept
either 404 as number or response.statusCode === 404) so that missing-object
errors are swallowed and other errors are rethrown; update the catch around the
bucket.file(...).delete() call accordingly and avoid relying on a top-level
string '404'.
| const res = await fetch(url, { | ||
| method: 'PUT', | ||
| headers: { | ||
| Authorization: `Bearer ${apiKey}`, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ status }), | ||
| }); | ||
| console.log(`Instatus report: ${status} → HTTP ${res.status}`); | ||
| } catch (err) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Instatus v2 component update endpoints, are non-2xx responses considered failed updates that clients should explicitly handle?
💡 Result:
Yes. For Instatus v2 component update endpoints (PUT /v2/:page_id/components/:component_id), you should treat any non-2xx HTTP response as a failed update that clients must handle explicitly. Instatus’s API documentation states that “Responses from the Instatus API, including errors, are encoded as JSON” and that error responses include a code and message in the payload (so clients should not assume that only the request transport succeeded) [1]. For component updates, the API reference documents the update endpoint and shows a normal 200-style success example response when the update succeeds [2][3]. Given that Instatus uses explicit error payloads with an “error” object (code/message) and that non-2xx status codes are categorized by HTTP as client/server errors rather than successful responses [4], a non-2xx response indicates the server did not complete the update. Practical client guidance: consider the update successful only when the HTTP status is in the 2xx range and you can parse the success response body; otherwise, read the JSON error payload (error.code / error.message) and handle/retry/alert as appropriate [1].
Citations:
- 1: https://instatus.com/help/api
- 2: https://help.instatus.com/help/api/components
- 3: https://instatus.com/help/api/components
- 4: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced file around the stated lines
ls -la
sed -n '1,120p' wwebjs/smoke/instatus.ts
# Check for any other fetch/error handling patterns in the repo
rg -n "instatus.*fetch|fetch\\(url|res\\.ok|HTTP .*res\\.status|reported|Instatus report" wwebjs/smoke/instatus.ts wwebjs -SRepository: glific/cypress-testing
Length of output: 2434
Treat non-2xx Instatus responses as failures.
reportToInstatus logs success for any HTTP status and only treats network/transport errors as failures (the catch won’t run for 401/404/5xx). Check res.ok and fail the report on non-2xx.
Suggested fix
const res = await fetch(url, {
method: 'PUT',
headers: {
Authorization: `Bearer ${apiKey}`,
'Content-Type': 'application/json',
},
body: JSON.stringify({ status }),
});
- console.log(`Instatus report: ${status} → HTTP ${res.status}`);
+ if (!res.ok) {
+ throw new Error(`Instatus report failed with HTTP ${res.status}`);
+ }
+ console.log(`Instatus report: ${status} → HTTP ${res.status}`);For Instatus v2 component update endpoints, are non-2xx responses considered failed updates that clients should explicitly handle?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wwebjs/smoke/instatus.ts` around lines 14 - 23, The function reportToInstatus
currently logs success for any HTTP response and only treats transport errors as
failures; update reportToInstatus to check the fetch Response (res) using res.ok
after the PUT and treat non-2xx responses as failures by logging an error and
throwing or returning a rejected result so callers can handle it; locate the
fetch call and the console.log("Instatus report...") that follows it, replace or
augment that logic to inspect res.ok (and optionally include
res.status/res.statusText and body) and handle non-ok responses as errors.
| function waitForRemoteSessionSaved(client: Client, timeoutMs: number): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| const timer = setTimeout(() => { | ||
| reject(new Error(`Session was not saved to GCS within ${timeoutMs / 1000}s`)); | ||
| }, timeoutMs); | ||
|
|
||
| client.once('remote_session_saved', () => { | ||
| clearTimeout(timer); | ||
| console.log('Remote session saved to GCS'); | ||
| resolve(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Avoid missing remote_session_saved due late listener registration.
The listener is attached only in finally; if the event already fired earlier, this path waits the full timeout and emits a false warning.
Suggested fix
+ let remoteSessionSaved = false;
+ client.on('remote_session_saved', () => {
+ remoteSessionSaved = true;
+ console.log('Remote session saved to GCS');
+ });
...
- if (!sessionAlreadyInGcs) {
+ if (!sessionAlreadyInGcs && !remoteSessionSaved) {
try {
await waitForRemoteSessionSaved(client, 90_000);
} catch (err) {
console.warn('Session may not have been uploaded to GCS:', (err as Error).message);
}
}Also applies to: 222-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wwebjs/smoke/runner.ts` around lines 27 - 38, The waitForRemoteSessionSaved
Promise attaches its 'remote_session_saved' listener too late and can miss
events that fired earlier; update waitForRemoteSessionSaved to first check an
existing saved-state on the client (e.g., a boolean property or getter like
client.remoteSessionSaved or client.isRemoteSessionSaved(), add that property if
it doesn't exist), resolve immediately if true, otherwise attach
client.once('remote_session_saved', ...) and start the timeout; also ensure any
other usage sites that currently add the listener in finally follow the same
pattern (check saved-state first then attach) so the event cannot be missed.
| if (!env.gcsBucket) { | ||
| console.error('GCS_BUCKET env var is required'); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Fail fast on missing bot/session configuration, not only bucket.
Line 145 validates only gcsBucket; missing botPhoneNumber or sessionId fails later with less actionable errors (for example invalid chat id).
Suggested fix
if (!env.gcsBucket) {
console.error('GCS_BUCKET env var is required');
process.exit(1);
}
+ if (!env.botPhoneNumber) {
+ console.error('BOT_PHONE_NUMBER env var is required');
+ process.exit(1);
+ }
+ if (!env.sessionId) {
+ console.error('WHATSAPP_SESSION_ID env var is required');
+ process.exit(1);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!env.gcsBucket) { | |
| console.error('GCS_BUCKET env var is required'); | |
| process.exit(1); | |
| } | |
| if (!env.gcsBucket) { | |
| console.error('GCS_BUCKET env var is required'); | |
| process.exit(1); | |
| } | |
| if (!env.botPhoneNumber) { | |
| console.error('BOT_PHONE_NUMBER env var is required'); | |
| process.exit(1); | |
| } | |
| if (!env.sessionId) { | |
| console.error('WHATSAPP_SESSION_ID env var is required'); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wwebjs/smoke/runner.ts` around lines 145 - 148, The runner currently only
validates env.gcsBucket and exits on missing value; add the same fail-fast
checks for env.botPhoneNumber and env.sessionId so missing bot/session
configuration is reported immediately. Update the validation block (the check
around env.gcsBucket in wwebjs/smoke/runner.ts) to validate env.gcsBucket,
env.botPhoneNumber and env.sessionId, using console.error() with actionable
messages for each missing variable and process.exit(1) to stop startup when any
are absent.
1 participant
Summary by CodeRabbit
New Features
Chores