-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Components] blocknative #10918 #19023
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds three new Blocknative action modules, extends the Blocknative app with HTTP helpers, API methods, and propDefinitions, bumps package version and dependency, and applies EOF newline fixes to multiple component files. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as Action Module
participant App as blocknative.app
participant API as Blocknative API
User->>Action: Trigger action (Get Chains / Get Gas Prices / Get Oracles)
Action->>Action: Read props (e.g., confidenceLevels, chainid)
Action->>App: Call corresponding method (getChains/getBlockprices/getOracles)
App->>App: Build request (base URL + Authorization) via _makeRequest
App->>API: HTTP GET (e.g., /chains, /gasprices/blockprices, /oracles)
API-->>App: Return data
App-->>Action: Response payload
Action->>Action: $.export("$summary", "Successfully retrieved X results")
Action-->>User: Return results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
components/blocknative/actions/get-chains/get-chains.mjs(1 hunks)components/blocknative/actions/get-gas-prices/get-gas-prices.mjs(1 hunks)components/blocknative/actions/get-oracles/get-oracles.mjs(1 hunks)components/blocknative/blocknative.app.mjs(1 hunks)components/blocknative/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/blocknative/package.json
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/blocknative/blocknative.app.mjs
🧬 Code graph analysis (4)
components/blocknative/actions/get-chains/get-chains.mjs (2)
components/blocknative/actions/get-oracles/get-oracles.mjs (1)
response(18-20)components/blocknative/blocknative.app.mjs (1)
response(17-17)
components/blocknative/actions/get-oracles/get-oracles.mjs (1)
components/blocknative/actions/get-chains/get-chains.mjs (1)
response(18-20)
components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (3)
components/blocknative/actions/get-chains/get-chains.mjs (1)
response(18-20)components/blocknative/actions/get-oracles/get-oracles.mjs (1)
response(18-20)components/blocknative/blocknative.app.mjs (1)
response(17-17)
components/blocknative/blocknative.app.mjs (3)
components/blocknative/actions/get-chains/get-chains.mjs (1)
response(18-20)components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (1)
response(30-36)components/blocknative/actions/get-oracles/get-oracles.mjs (1)
response(18-20)
⏰ 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: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/blocknative/package.json (1)
3-16: Dependency bump aligns with new platform usage.Good call adding the @pipedream/platform dependency with the version bump—this matches the new axios-based helper in the app and keeps everything consistent.
components/blocknative/actions/get-oracles/get-oracles.mjs (1)
3-24: Action wiring looks solid.The action cleanly delegates to the app helper and returns the raw list, so consumers can decide how to shape the data—looks good.
components/blocknative/actions/get-chains/get-chains.mjs (1)
3-24: Implementation matches the API contract.This mirrors the oracles action, and the summary reads well—nothing to fix here.
components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (1)
3-39: Gas price action behaves as expected.The prop wiring and params serialization match Blocknative’s requirements; exporting the estimated price count is a nice touch.
GTFalcao
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.
LGTM!
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
06f2ac7 to
e8f5e95
Compare
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: 4
♻️ Duplicate comments (1)
components/blocknative/blocknative.app.mjs (1)
31-46: Authorization header issue remains unresolved.The issue flagged in the previous review is still present: Line 42 unconditionally sets the Authorization header to
${this.$auth.api_key}, which results inAuthorization: undefinedwhen no API key is provided. This breaks the optional authentication path that Blocknative's public endpoints support.Please apply the suggested fix from the previous review to make the header conditional:
- return axios($, { - ...otherOpts, - url: this._baseUrl() + path, - headers: { - "Authorization": `${this.$auth.api_key}`, - ...headers, - }, - }); + const authHeader = this.$auth?.api_key + ? { Authorization: this.$auth.api_key } + : {}; + + return axios($, { + ...otherOpts, + url: this._baseUrl() + path, + headers: { + ...authHeader, + ...headers, + }, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
components/blocknative/actions/get-chains/get-chains.mjs(1 hunks)components/blocknative/actions/get-gas-prices/get-gas-prices.mjs(1 hunks)components/blocknative/actions/get-oracles/get-oracles.mjs(1 hunks)components/blocknative/blocknative.app.mjs(1 hunks)components/blocknative/package.json(2 hunks)components/browseract/browseract.app.mjs(1 hunks)components/camino_ai/camino_ai.app.mjs(1 hunks)components/clappia/clappia.app.mjs(1 hunks)components/digitalocean_gradient_ai_serverless_inference/digitalocean_gradient_ai_serverless_inference.app.mjs(1 hunks)components/elastic_security/elastic_security.app.mjs(1 hunks)components/postnitro/postnitro.app.mjs(1 hunks)components/redash/redash.app.mjs(1 hunks)components/spike/spike.app.mjs(1 hunks)components/straico/straico.app.mjs(1 hunks)components/straker_verify/straker_verify.app.mjs(1 hunks)components/trackvia/trackvia.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/blocknative/blocknative.app.mjs
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/blocknative/package.json
🧬 Code graph analysis (4)
components/blocknative/actions/get-oracles/get-oracles.mjs (1)
components/blocknative/actions/get-chains/get-chains.mjs (1)
response(18-20)
components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (3)
components/blocknative/actions/get-chains/get-chains.mjs (1)
response(18-20)components/blocknative/actions/get-oracles/get-oracles.mjs (1)
response(18-20)components/blocknative/blocknative.app.mjs (1)
response(17-17)
components/blocknative/blocknative.app.mjs (3)
components/blocknative/actions/get-chains/get-chains.mjs (1)
response(18-20)components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (1)
response(30-36)components/blocknative/actions/get-oracles/get-oracles.mjs (1)
response(18-20)
components/blocknative/actions/get-chains/get-chains.mjs (3)
components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (1)
response(30-36)components/blocknative/actions/get-oracles/get-oracles.mjs (1)
response(18-20)components/blocknative/blocknative.app.mjs (1)
response(17-17)
⏰ 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: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (10)
components/straker_verify/straker_verify.app.mjs (1)
11-11: LGTM!Trailing newline added following best practices.
components/camino_ai/camino_ai.app.mjs (1)
11-11: LGTM!Trailing newline added following best practices.
components/postnitro/postnitro.app.mjs (1)
11-11: LGTM!Trailing newline added following best practices.
components/blocknative/package.json (1)
3-3: LGTM!Version bump to 0.1.0 is appropriate for the new feature additions. The @pipedream/platform dependency correctly supports the axios usage introduced in blocknative.app.mjs.
Also applies to: 15-17
components/blocknative/blocknative.app.mjs (3)
1-2: LGTM!Import statement correctly brings in axios from the @pipedream/platform dependency added in package.json.
28-30: LGTM!Base URL method follows standard pattern.
48-65: LGTM!The API methods (
getBlockprices,getChains,getOracles) follow a consistent pattern and correctly delegate to_makeRequest. They properly match the usage patterns in the new action modules.components/blocknative/actions/get-oracles/get-oracles.mjs (1)
1-16: LGTM!The action metadata and configuration are well-structured. The annotations correctly indicate this is a read-only, non-destructive operation.
components/blocknative/actions/get-chains/get-chains.mjs (1)
1-16: LGTM!The action metadata and configuration follow the established pattern and correctly describe a read-only operation.
components/blocknative/actions/get-gas-prices/get-gas-prices.mjs (1)
1-28: LGTM!The action configuration is well-structured, and the use of
propDefinitionfor reusable props is a good practice.
| async run({ $ }) { | ||
| const response = await this.app.getChains({ | ||
| $, | ||
| }); | ||
| $.export("$summary", "Successfully retrieved " + response.length + " chains"); | ||
| return response; | ||
| }, |
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.
Add validation for response structure.
The code accesses response.length without validating that the response is an array, which could cause a runtime error if the API returns an unexpected structure.
Apply this diff to add validation:
async run({ $ }) {
const response = await this.app.getChains({
$,
});
- $.export("$summary", "Successfully retrieved " + response.length + " chains");
+ if (!Array.isArray(response)) {
+ throw new Error("Unexpected response format: expected an array");
+ }
+ $.export("$summary", `Successfully retrieved ${response.length} chains`);
return response;
},Additionally, prefer template literals over string concatenation.
🤖 Prompt for AI Agents
In components/blocknative/actions/get-chains/get-chains.mjs around lines 17 to
23, the code assumes response is an array and uses response.length and string
concatenation; update it to validate that response is an array before using
length (e.g., if not an array, log/export a clear error or return an empty
array/throw), ensure downstream returns a predictable value, and change the
summary message to use a template literal for readability (e.g., include the
validated length variable in a backtick string).
| async run({ $ }) { | ||
| const response = await this.app.getBlockprices({ | ||
| $, | ||
| params: { | ||
| confidenceLevels: this.confidenceLevels.join(","), | ||
| chainid: this.chainid, | ||
| }, | ||
| }); | ||
| $.export("$summary", "Successfully retrieved " + response.blockPrices[0].estimatedPrices.length + " estimated prices"); | ||
| return response; | ||
| }, |
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.
Add validation for nested response structure.
Line 37 accesses deeply nested properties (response.blockPrices[0].estimatedPrices.length) without validation. This will throw a runtime error if:
response.blockPricesis undefined or not an arrayresponse.blockPricesis emptyresponse.blockPrices[0]doesn't haveestimatedPrices
Apply this diff to add proper validation:
async run({ $ }) {
const response = await this.app.getBlockprices({
$,
params: {
confidenceLevels: this.confidenceLevels.join(","),
chainid: this.chainid,
},
});
- $.export("$summary", "Successfully retrieved " + response.blockPrices[0].estimatedPrices.length + " estimated prices");
+ if (!response.blockPrices?.length || !response.blockPrices[0]?.estimatedPrices) {
+ throw new Error("Unexpected response format: missing blockPrices or estimatedPrices");
+ }
+ $.export("$summary", `Successfully retrieved ${response.blockPrices[0].estimatedPrices.length} estimated prices`);
return response;
},Additionally, prefer template literals over string concatenation.
🤖 Prompt for AI Agents
In components/blocknative/actions/get-gas-prices/get-gas-prices.mjs around lines
29 to 39, the code assumes response.blockPrices[0].estimatedPrices exists and
reads its length directly; add defensive checks to ensure response is an object,
response.blockPrices is an array with at least one element, and
response.blockPrices[0].estimatedPrices is an array before accessing .length,
falling back to 0 (or a safe default) if any check fails; then use a template
literal for the summary export (e.g. `Successfully retrieved ${count} estimated
prices`) and keep returning the original response.
| async run({ $ }) { | ||
| const response = await this.app.getOracles({ | ||
| $, | ||
| }); | ||
| $.export("$summary", "Successfully retrieved " + response.length + " results"); | ||
| return response; | ||
| }, |
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.
Add validation for response structure.
The code accesses response.length without validating that the response is an array. If the API returns an unexpected structure, this will cause a runtime error.
Consider adding validation:
async run({ $ }) {
const response = await this.app.getOracles({
$,
});
- $.export("$summary", "Successfully retrieved " + response.length + " results");
+ if (!Array.isArray(response)) {
+ throw new Error("Unexpected response format: expected an array");
+ }
+ $.export("$summary", `Successfully retrieved ${response.length} results`);
return response;
},Additionally, prefer template literals over string concatenation for improved readability.
📝 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.
| async run({ $ }) { | |
| const response = await this.app.getOracles({ | |
| $, | |
| }); | |
| $.export("$summary", "Successfully retrieved " + response.length + " results"); | |
| return response; | |
| }, | |
| async run({ $ }) { | |
| const response = await this.app.getOracles({ | |
| $, | |
| }); | |
| if (!Array.isArray(response)) { | |
| throw new Error("Unexpected response format: expected an array"); | |
| } | |
| $.export("$summary", `Successfully retrieved ${response.length} results`); | |
| return response; | |
| }, |
🤖 Prompt for AI Agents
In components/blocknative/actions/get-oracles/get-oracles.mjs around lines 17 to
23, the code assumes response is an array and uses response.length and string
concatenation; validate the response shape first (e.g. if
(!Array.isArray(response)) handle it by either throwing a descriptive error or
normalizing to an empty array), then use the array's length safely and export
the summary using a template literal (e.g. `\`Successfully retrieved
${results.length} results\``); ensure the function still returns the normalized
value so callers never receive an unexpected structure.
WHY
Summary by CodeRabbit
New Features
Chores
Style