-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[FIX] Lusha API Authentication Issue #18889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughVersion bumps across multiple Lusha actions and package, mapping changes from single Changes
Sequence Diagram(s)sequenceDiagram
participant Action as CompanySearch.run()
participant Builder as include builder
participant Lusha as lusha.searchCompanies
participant Caller as Action caller
Note over Action,Builder: Build conditional include object
Action->>Builder: gather filters (names, domains, locations, sizes, revenues, sicCodes, naicsCodes)
Builder-->>Action: include (only provided fields, locations -> [{country}...])
Action->>Lusha: lusha.searchCompanies({ include })
Lusha-->>Action: response
Action->>Caller: return response
sequenceDiagram
participant Caller
participant Paginate as paginate()
participant API as Lusha API
Note over Caller,Paginate: New paginate signature & flow
Caller->>Paginate: paginate({ fn, data={}, maxResults })
Paginate->>Paginate: payload.pages = data.pages
Paginate->>API: _makeRequest(payload with data)
API-->>Paginate: response
Paginate->>Paginate: results = response.data → iterate results
Paginate-->>Caller: accumulated results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs (1)
90-108: Critical: Pagination broken due to params vs data mismatch.The manual pagination loop passes pagination info via
params.pages(lines 95-100), but based on the refactoredpaginatemethod inlusha.app.mjs(line 272), pagination should now be indata.pages. The Lusha API likely expects pagination in the request body, not query parameters.Apply this diff to fix the pagination:
do { const { requestId, data = [], } = await this.lusha.searchContacts({ $, - params: { - pages: { - page, - size: 50, - }, - }, data: { + pages: { + page, + size: 50, + }, filters: { contacts: { include, }, }, }, });components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (1)
80-98: Critical: Pagination broken due to params vs data mismatch.The manual pagination loop passes pagination info via
params.pages(lines 85-90), but the refactoredpaginatemethod inlusha.app.mjs(line 272) now usesdata.pages. This inconsistency will cause pagination to fail.Apply this diff to fix the pagination:
do { const { requestId, data = [], } = await this.lusha.searchCompanies({ $, - params: { - pages: { - page, - size: 50, - }, - }, data: { + pages: { + page, + size: 50, + }, filters: { companies: { include, }, }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/lusha/actions/company-enrich/company-enrich.mjs(1 hunks)components/lusha/actions/company-search/company-search.mjs(1 hunks)components/lusha/actions/contact-enrich/contact-enrich.mjs(1 hunks)components/lusha/actions/contact-search/contact-search.mjs(1 hunks)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs(1 hunks)components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs(1 hunks)components/lusha/lusha.app.mjs(2 hunks)components/lusha/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/lusha/lusha.app.mjs (4)
components/lusha/actions/company-enrich/company-enrich.mjs (1)
response(32-38)components/lusha/actions/company-search/company-search.mjs (1)
response(78-89)components/lusha/actions/contact-enrich/contact-enrich.mjs (1)
response(32-38)components/lusha/actions/contact-search/contact-search.mjs (1)
response(87-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/lusha/lusha.app.mjs (1)
182-182: Verify whether hardcoded debug mode is acceptable for production use and whether API key exposure is a known/acceptable trade-off.The Pipedream platform's debug feature intentionally exports the full request configuration (including the
Authorizationheader with the API key) tostep.debug_configwhen enabled. ThecloneSafe()function performs only a deep clone without filtering sensitive data.This hardcoding is atypical compared to how most components use the debug flag (primarily in source/webhook handlers). Since the original concern about API key exposure is technically valid, confirm whether this debug mode is:
- Required for troubleshooting in production
- An oversight that should be removed or made configurable
- An accepted security/observability trade-off for this integration
If debug is not essential for production use, consider removing it or gating it behind an environment variable or component setting.
luancazarine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jcortes, LGTM! Ready for QA!
For Integration QA: |
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
b0364a7 to
2d2a242
Compare
|
HI @vunguyenhung about: lusha - Company Search - FailedCompany search by name (Regression) - FailedImprovements neededReturn value should include the requestId and other metadata, for the user to use on other actions. AnswerThis is not a good idea because we are using pagination so we end up with a requestId per pagination lusha - Contact Search - FailedImprovement needed - Improve
|
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
1 similar comment
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
2d2a242 to
91b8442
Compare
|
Hi @vunguyenhung I've just pushed some changes please test again. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs (1)
95-99: Reduce page size to Lusha’s documented maximum.The Prospecting Search API caps
pages.sizeat 40. Sending 50 results in a validation error from Lusha, so the loop will fail before hitting the enrich step. Clamp this to 40 (or make it configurable within the allowed 10–40 window). (lusha.com)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (1)
152-168: Handle empty search pages before enriching.When a search page returns zero companies,
companiesIdsstays empty but we still call/prospecting/company/enrich. Lusha’s API requires at least one ID and responds with 400 otherwise, so a valid “no results” search will now fail. Add a guard to skip the enrichment call whencompaniesIdsis empty.const companiesIds = []; for (const d of data) { companiesIds.push(d.id); if (++count >= this.limit) { hasMore = false; break; } } + if (!companiesIds.length) { + break; + } + const enrichedCompanies = await this.lusha.enrichCompanies({ $, data: { requestId, companiesIds, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
components/lusha/actions/company-enrich/company-enrich.mjs(1 hunks)components/lusha/actions/company-search/company-search.mjs(2 hunks)components/lusha/actions/contact-enrich/contact-enrich.mjs(1 hunks)components/lusha/actions/contact-search/contact-search.mjs(2 hunks)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs(4 hunks)components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs(2 hunks)components/lusha/lusha.app.mjs(3 hunks)components/lusha/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/lusha/lusha.app.mjs
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: LucBerge
Repo: PipedreamHQ/pipedream PR: 14080
File: components/nocodb/nocodb.app.mjs:133-133
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When implementing pagination with an offset, incrementing `args.params.offset` within the loop ensures correct tracking of the offset, particularly when a maximum count limit (`max`) is used.
Applied to files:
components/lusha/lusha.app.mjs
🧬 Code graph analysis (5)
components/lusha/lusha.app.mjs (4)
components/lusha/actions/company-enrich/company-enrich.mjs (1)
response(32-38)components/lusha/actions/company-search/company-search.mjs (1)
response(123-132)components/lusha/actions/contact-enrich/contact-enrich.mjs (1)
response(32-38)components/lusha/actions/contact-search/contact-search.mjs (1)
response(87-98)
components/lusha/actions/contact-search/contact-search.mjs (2)
components/lusha/actions/company-search/company-search.mjs (1)
include(72-117)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (1)
include(78-123)
components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs (2)
components/lusha/actions/company-search/company-search.mjs (1)
include(72-117)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (1)
include(78-123)
components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (2)
components/lusha/actions/company-search/company-search.mjs (1)
include(72-117)components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs (2)
include(75-75)hasMore(88-88)
components/lusha/actions/company-search/company-search.mjs (3)
components/lusha/actions/contact-search/contact-search.mjs (2)
include(75-75)response(87-98)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (2)
include(78-123)lusha(133-150)components/lusha/lusha.app.mjs (5)
sizes(31-31)revenues(41-41)sicCodes(51-51)naicsCodes(66-66)response(294-297)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/lusha/actions/company-enrich/company-enrich.mjs (1)
7-7: Version bump acknowledged.No functional code changes here, and the metadata update keeps the action aligned with the rest of the package.
| data.pages = { | ||
| page: ++page, | ||
| size: 50, | ||
| }; | ||
| const { data } = await fn({ | ||
| params, | ||
| const response = await fn({ | ||
| data, | ||
| ...opts, | ||
| }); | ||
| for (const d of data) { | ||
| const results = response.data || []; | ||
| for (const d of results) { | ||
| yield d; | ||
|
|
||
| if (maxResults && ++count === maxResults) { | ||
| return count; | ||
| } | ||
| } | ||
|
|
||
| hasMore = data.length; | ||
| hasMore = results.length; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix paginator page size (Lusha rejects 50).
The helper now hardcodes pages.size = 50, but Lusha’s API only accepts 10–40. Any request through this paginator will 400, which regresses every action that relies on it. Please drop this to 40 (or enforce the documented bounds dynamically) before the release. (lusha.com)
🤖 Prompt for AI Agents
In components/lusha/lusha.app.mjs around lines 290 to 308, the paginator sets
pages.size = 50 which Lusha's API rejects (valid range 10–40); change the
hardcoded value to 40 or enforce bounds dynamically (e.g., clamp requested size
to between 10 and 40 before assigning pages.size) so requests use an accepted
page size and avoid 400 errors.
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
WHY
Resolves #18861
Summary by CodeRabbit
Chores
Improvements