🌿 Fern Regeneration -- May 5, 2026#249
Merged
Merged
Conversation
✅ Actions performedReview triggered.
|
Collaborator
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR performs a coordinated SDK evolution: upgrading to version 1.2.0, refactoring authentication from builder patterns to dedicated factory methods with token and OAuth credential flows, standardizing HTTP response handling to avoid multiple body reads, adding RequestOptions overloads across resource clients, introducing bidirectional pagination interfaces, enhancing retry logic with header-based backoff, and replacing the okhttp3.mockwebserver test dependency with custom implementations. ChangesSDK Version Upgrade & Authentication Refactoring
HTTP Response & Error Handling Standardization
RequestOptions API Expansion
Pagination & Filter Types
Test Infrastructure Replacement
sequenceDiagram
participant Client as Client Code
participant Builder as BaseClientBuilder
participant TokenAuth as _TokenAuth
participant CredAuth as _CredentialsAuth
participant OAuthClient as OauthTokensClient
participant OAuthSupplier as OAuthTokenSupplier
participant HttpClient as HttpClient
participant Interceptor as RetryInterceptor
Client->>+Builder: withToken(token)
Builder->>+TokenAuth: new _TokenAuth(token)
TokenAuth-->>-Builder: instance
Builder-->>-Client: _TokenAuth builder
Client->>+TokenAuth: build()
TokenAuth->>+HttpClient: setHeader(Authorization: Bearer)
HttpClient-->>-TokenAuth: configured client
TokenAuth-->>-Client: BaseClient with token auth
Client->>+Builder: withCredentials(clientId, clientSecret)
Builder->>+CredAuth: new _CredentialsAuth(clientId, clientSecret)
CredAuth-->>-Builder: instance
Builder-->>-Client: _CredentialsAuth builder
Client->>+CredAuth: scope(scope).build()
CredAuth->>+OAuthClient: create OAuth token request
OAuthClient->>+OAuthSupplier: new OAuthTokenSupplier(id, secret, scope, client)
OAuthSupplier-->>-OAuthClient: supplier
OAuthClient->>+HttpClient: setHeader(Authorization: Bearer)
HttpClient-->>-OAuthClient: client with OAuth token
OAuthClient-->>-CredAuth: response
CredAuth-->>-Client: BaseClient with OAuth auth
Client->>+HttpClient: execute request
HttpClient->>+Interceptor: retry on 429/timeout
Interceptor->>Interceptor: check Retry-After header
Interceptor->>Interceptor: calculate backoff with jitter
Interceptor->>+HttpClient: retry request
HttpClient-->>-Interceptor: response
Interceptor-->>-Client: final response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/pipedream/api/resources/tokens/RawTokensClient.java (1)
78-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlways throw
TooManyRequestsErrorfor HTTP 429, regardless of body parseability.Right now, a non-JSON 429 body downgrades to
BaseClientApiException. That makes retry/catch behavior depend on payload format instead of status code.💡 Suggested fix
- try { - if (response.code() == 429) { - throw new TooManyRequestsError( - ObjectMappers.JSON_MAPPER.readValue(responseBodyString, Object.class), response); - } - } catch (JsonProcessingException ignored) { - // unable to map error response, throwing generic error - } Object errorBody = ObjectMappers.parseErrorBody(responseBodyString); + if (response.code() == 429) { + throw new TooManyRequestsError(errorBody, response); + } throw new BaseClientApiException( "Error with status code " + response.code(), response.code(), errorBody, response);Apply the same change in both
create(...)andvalidate(...).Also applies to: 137-147
🤖 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 `@src/main/java/com/pipedream/api/resources/tokens/RawTokensClient.java` around lines 78 - 88, The 429 handling in RawTokensClient currently falls back to BaseClientApiException if the response body can't be JSON-parsed; update the 429 branch in both create(...) and validate(...) so that any HTTP 429 always throws TooManyRequestsError: detect response.code() == 429 first, attempt to parse the body (using ObjectMappers.JSON_MAPPER.readValue or ObjectMappers.parseErrorBody) but if parsing fails use a safe fallback (raw responseBodyString or null) and construct/throw TooManyRequestsError with that fallback and the original response instead of letting the JsonProcessingException cause a generic BaseClientApiException.src/main/java/com/pipedream/api/core/RetryInterceptor.java (1)
22-26:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMake the retry counter request-scoped.
retryNumberis stored on a singleExponentialBackoffinstance hanging off the interceptor, so all requests share the same mutable retry budget. After one call burns through retries, later calls can stop retrying immediately, and concurrent calls race each other on the same counter.Possible fix
-public class RetryInterceptor implements Interceptor { - - private final ExponentialBackoff backoff; +public class RetryInterceptor implements Interceptor { + + private final int maxRetries; private final Random random = new Random(); public RetryInterceptor(int maxRetries) { - this.backoff = new ExponentialBackoff(maxRetries); + this.maxRetries = maxRetries; } @@ private Response retryChain(Response response, Chain chain) throws IOException { - Optional<Duration> nextBackoff = this.backoff.nextBackoff(response); + ExponentialBackoff backoff = new ExponentialBackoff(maxRetries); + Optional<Duration> nextBackoff = backoff.nextBackoff(response);Based on learnings: "If changes are needed to generated files, changes must happen upstream in the Fern API definition or generator config and arrive via regeneration."
Also applies to: 160-177
🤖 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 `@src/main/java/com/pipedream/api/core/RetryInterceptor.java` around lines 22 - 26, The ExponentialBackoff instance (field backoff) currently holds a shared mutable retry counter used across all requests; change RetryInterceptor to make the retry counter request-scoped by removing the backoff field and constructing a fresh ExponentialBackoff inside the per-request flow (e.g., inside the intercept method or the retry loop) so each request gets its own retryNumber; update the constructor (which currently sets this.backoff) to no longer store backoff and ensure any code referencing the field (including uses around lines 160-177) instead uses the new local ExponentialBackoff instance created for that request.
🤖 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 `@src/main/java/com/pipedream/api/AsyncBaseClientBuilder.java`:
- Around line 267-309: The nested auth builder classes _TokenAuth and
_CredentialsAuth must be generic-specialized so fluent chaining returns the
subclass type: change their inheritance from AsyncBaseClientBuilder to
AsyncBaseClientBuilder<_TokenAuth> and AsyncBaseClientBuilder<_CredentialsAuth>
respectively so methods like scope(), projectId(), etc. return the correct
subtype; apply this change in the Fern API definition or generator config and
regenerate the file so the generated classes preserve fluent chaining for those
builders.
In `@src/main/java/com/pipedream/api/BaseClientBuilder.java`:
- Around line 267-309: The nested auth builders _TokenAuth and _CredentialsAuth
currently extend the raw BaseClientBuilder which causes inherited fluent methods
(projectId(), url(), timeout(), etc.) to return BaseClientBuilder and break
chaining; change their declarations to extend BaseClientBuilder<_TokenAuth> and
BaseClientBuilder<_CredentialsAuth> respectively (and do the same for the
corresponding nested auth classes in AsyncBaseClientBuilder), ensuring the class
generic parameter matches the concrete subclass so methods like
withCredentials(...).projectId(...).scope(...) continue to return the correct
subtype and preserve fluent chaining.
In `@src/main/java/com/pipedream/api/core/ClientOptions.java`:
- Around line 191-198: Builder.from(...) currently copies an existing
OkHttpClient but build() only installs RetryInterceptor when httpClient was
null, so maxRetries changes are ignored; to fix, always obtain an
OkHttpClient.Builder (either this.httpClient.newBuilder() or new
OkHttpClient.Builder()) inside ClientOptions.build(), remove any existing
RetryInterceptor instances from its interceptors, and then add a new
RetryInterceptor configured with this.maxRetries (skipping if maxRetries
indicates "no retries"). This ensures changes made via Builder.maxRetries(...)
or Builder.httpClient(...) are reflected in the resulting OkHttpClient
regardless of whether an existing client was provided.
In `@src/main/java/com/pipedream/api/core/pagination/AsyncCustomPager.java`:
- Around line 70-76: The three async stub methods createAsync, getAllItemsAsync,
and forEachPageAsync currently throw UnsupportedOperationException
synchronously; change them to return a CompletableFuture that is already
completed exceptionally with the same UnsupportedOperationException (use
CompletableFuture.failedFuture or create a new CompletableFuture and call
completeExceptionally) so callers observe the error via the returned future;
mirror the pattern used in nextPageAsync/previousPageAsync to route errors
through the future rather than throwing synchronously.
In `@src/main/java/com/pipedream/api/core/pagination/CustomPager.java`:
- Around line 69-74: Update the UnsupportedOperationException message in the
static factory method CustomPager.create to point to the correct file path
core/pagination/CustomPager.java (instead of core/CustomPager.java); modify the
exception text in the create(Object initialResponse, ClientOptions
clientOptions, Object requestOptions) method of class CustomPager so it
instructs users to implement the class at core/pagination/CustomPager.java and
retains the rest of the guidance about .fernignore and implementation examples.
In `@src/main/java/com/pipedream/api/PipedreamClientBuilder.java`:
- Around line 17-37: The build() method currently allows partial OAuth
credentials to silently fall through; add an explicit validation inside build()
(before creating baseOptions) that if either clientId or clientSecret is
non-null while the other is null, throw an IllegalStateException with a clear
message (e.g., "Both clientId and clientSecret must be provided for OAuth or
neither."); keep using buildClientOptions(), and preserve the existing branches
that create OauthTokensClient and OAuthTokenSupplier when both credentials are
present and the token branch when token is provided.
In `@src/main/java/com/pipedream/api/resources/accounts/AccountsClient.java`:
- Around line 38-43: The public API introduces ambiguous overloads—specifically
list(RequestOptions) vs list(AccountsListRequest) and retrieve(String,
RequestOptions) vs retrieve(String, AccountsRetrieveRequest)—which breaks
callers that pass null; to fix, remove or rename the RequestOptions-only
overloads in AccountsClient (e.g., change list(RequestOptions) and
retrieve(String, RequestOptions) to distinct names such as
listWithRequestOptions and retrieveWithRequestOptions or deprecate them), update
all callers to use the new names or explicit typed arguments, and ensure
rawClient usage (rawClient.list/rawClient.retrieve) is adapted accordingly so no
two overloads accept null ambiguously.
In `@src/main/java/com/pipedream/api/resources/apps/AppsClient.java`:
- Around line 40-42: The new overload list(RequestOptions) in AppsClient
introduces ambiguity with the existing list(AppsListRequest) because calls like
list(null) are now ambiguous; update the Fern template/API definition to remove
or rename the generated overload (i.e., eliminate list(RequestOptions) or change
its signature) so only one list(...) overload remains in class AppsClient (and
ensure corresponding rawClient.list invocations and generated models reflect the
change) rather than hand-editing the generated AppsClient.java.
In `@src/main/java/com/pipedream/api/resources/apps/RawAppsClient.java`:
- Around line 42-47: The added single-argument overload public
BaseClientHttpResponse<SyncPagingIterable<App>> list(RequestOptions
requestOptions) in RawAppsClient introduces an ambiguous call with
list(AppsListRequest) (e.g., client.list(null)); remove this overload and either
keep the existing zero-arg helper list() and the two-arg list(AppsListRequest,
RequestOptions) or replace the removed overload with a non-colliding convenience
method name (e.g., listWithRequestOptions(RequestOptions)) so callers aren’t
broken; update callers if you add a renamed helper and, if this file is
generated, make the change upstream in the Fern API definition/generator config
so it persists on regeneration.
In
`@src/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.java`:
- Around line 56-61: The new overloads list(RequestOptions) and update(String,
RequestOptions) in AsyncRawProjectsClient collide with existing request-object
overloads and create ambiguous null call-sites; fix this in the Fern
API/generator template (not by hand) so it does not emit overloads that only
differ by a nullable RequestOptions parameter—either remove these overloads,
give the RequestOptions variants distinct names (e.g., listWithOptions /
updateWithOptions), or alter the signature shape (wrap RequestOptions in a
distinct type or make it a non-ambiguous last vararg) so calls like list(null)
and update(projectId, null) are unambiguous; update the generator rule that
produces the list(RequestOptions) and update(String, RequestOptions) signatures
in AsyncRawProjectsClient to follow one of these non-ambiguous patterns.
In `@src/main/java/com/pipedream/api/resources/projects/RawProjectsClient.java`:
- Around line 51-56: The new null-ambiguous overloads break source
compatibility: remove or revert the newly added overloads that accept only
RequestOptions (list(RequestOptions)) and only RequestOptions for update
(update(String, RequestOptions)), and instead ensure the Fern generator / API
definition upstream produces a single unambiguous method signature (keep
list(ProjectsListRequest) and update(String, UpdateProjectOpts) or give the
alternate methods distinct names). Locate the generated methods
list(RequestOptions), list(ProjectsListRequest), update(String, RequestOptions),
and update(String, UpdateProjectOpts) and remove or rename the ambiguous
RequestOptions-only overloads so calls like list(null) and update(id, null) no
longer cause compilation ambiguity; fix must be applied in the generator/spec
rather than ad-hoc in generated code.
In `@src/test/java/com/pipedream/api/OauthTokensWireTest.java`:
- Around line 25-27: The test currently calls
PipedreamClient.builder().url(...).build(), but builder() pulls
PIPEDREAM_CLIENT_ID/PIPEDREAM_CLIENT_SECRET from the environment and can trigger
a real OAuth flow; update the builder chain for the test to neutralize those env
defaults by explicitly setting .clientId(null).clientSecret(null) before calling
build() (i.e., use
PipedreamClient.builder().url(...).clientId(null).clientSecret(null).build());
because this is a Fern-generated test, also make the corresponding change in the
Fern generator/template (or add this test to .fernignore) so regenerated code
preserves the neutralized credentials.
In `@src/test/java/com/pipedream/api/ProxyClientWireTest.java`:
- Around line 36-45: The comment is wrong: the builders read
PIPEDREAM_CLIENT_ID/PIPEDREAM_CLIENT_SECRET from the environment and will
trigger OAuth; fix both client constructions (PipedreamClient.builder() and
AsyncPipedreamClient.builder()) by explicitly disabling env credentials via
.clientId(null).clientSecret(null) before
.url(baseUrl).projectId("test-project").build() so the wire tests do not attempt
token fetching.
In `@src/test/java/com/pipedream/api/testutil/MockWebServer.java`:
- Around line 70-90: The handle() method in MockWebServer can leak the
HttpExchange if sendResponseHeaders or writing the response body throws; wrap
the whole response handling logic (from after responses.poll() up to the current
exchange.close()) in a try { ... } finally { exchange.close(); } so
exchange.close() always runs, and ensure any early returns (e.g. when response
== null) still close the exchange by moving that null-check response handling
into the same try/finally or closing before returning; update references in
MockWebServer.handle() accordingly.
---
Outside diff comments:
In `@src/main/java/com/pipedream/api/core/RetryInterceptor.java`:
- Around line 22-26: The ExponentialBackoff instance (field backoff) currently
holds a shared mutable retry counter used across all requests; change
RetryInterceptor to make the retry counter request-scoped by removing the
backoff field and constructing a fresh ExponentialBackoff inside the per-request
flow (e.g., inside the intercept method or the retry loop) so each request gets
its own retryNumber; update the constructor (which currently sets this.backoff)
to no longer store backoff and ensure any code referencing the field (including
uses around lines 160-177) instead uses the new local ExponentialBackoff
instance created for that request.
In `@src/main/java/com/pipedream/api/resources/tokens/RawTokensClient.java`:
- Around line 78-88: The 429 handling in RawTokensClient currently falls back to
BaseClientApiException if the response body can't be JSON-parsed; update the 429
branch in both create(...) and validate(...) so that any HTTP 429 always throws
TooManyRequestsError: detect response.code() == 429 first, attempt to parse the
body (using ObjectMappers.JSON_MAPPER.readValue or ObjectMappers.parseErrorBody)
but if parsing fails use a safe fallback (raw responseBodyString or null) and
construct/throw TooManyRequestsError with that fallback and the original
response instead of letting the JsonProcessingException cause a generic
BaseClientApiException.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: afbba88c-6d69-49d3-8516-6043cb6d6d00
📒 Files selected for processing (96)
.fern/metadata.json.fernignoreCLAUDE.mdREADME.mdbuild.gradlesrc/main/java/com/pipedream/api/AsyncBaseClient.javasrc/main/java/com/pipedream/api/AsyncBaseClientBuilder.javasrc/main/java/com/pipedream/api/AsyncPipedreamClient.javasrc/main/java/com/pipedream/api/AsyncPipedreamClientBuilder.javasrc/main/java/com/pipedream/api/BaseClient.javasrc/main/java/com/pipedream/api/BaseClientBuilder.javasrc/main/java/com/pipedream/api/PipedreamClient.javasrc/main/java/com/pipedream/api/PipedreamClientBuilder.javasrc/main/java/com/pipedream/api/core/BaseClientApiException.javasrc/main/java/com/pipedream/api/core/ClientOptions.javasrc/main/java/com/pipedream/api/core/NullableNonemptyFilter.javasrc/main/java/com/pipedream/api/core/OAuthTokenSupplier.javasrc/main/java/com/pipedream/api/core/ObjectMappers.javasrc/main/java/com/pipedream/api/core/RetryInterceptor.javasrc/main/java/com/pipedream/api/core/pagination/AsyncCustomPager.javasrc/main/java/com/pipedream/api/core/pagination/BiDirectionalPage.javasrc/main/java/com/pipedream/api/core/pagination/CustomPager.javasrc/main/java/com/pipedream/api/resources/accounts/AccountsClient.javasrc/main/java/com/pipedream/api/resources/accounts/AsyncAccountsClient.javasrc/main/java/com/pipedream/api/resources/accounts/AsyncRawAccountsClient.javasrc/main/java/com/pipedream/api/resources/accounts/RawAccountsClient.javasrc/main/java/com/pipedream/api/resources/actions/ActionsClient.javasrc/main/java/com/pipedream/api/resources/actions/AsyncActionsClient.javasrc/main/java/com/pipedream/api/resources/actions/AsyncRawActionsClient.javasrc/main/java/com/pipedream/api/resources/actions/RawActionsClient.javasrc/main/java/com/pipedream/api/resources/appcategories/AsyncRawAppCategoriesClient.javasrc/main/java/com/pipedream/api/resources/appcategories/RawAppCategoriesClient.javasrc/main/java/com/pipedream/api/resources/apps/AppsClient.javasrc/main/java/com/pipedream/api/resources/apps/AsyncAppsClient.javasrc/main/java/com/pipedream/api/resources/apps/AsyncRawAppsClient.javasrc/main/java/com/pipedream/api/resources/apps/RawAppsClient.javasrc/main/java/com/pipedream/api/resources/components/AsyncComponentsClient.javasrc/main/java/com/pipedream/api/resources/components/AsyncRawComponentsClient.javasrc/main/java/com/pipedream/api/resources/components/ComponentsClient.javasrc/main/java/com/pipedream/api/resources/components/RawComponentsClient.javasrc/main/java/com/pipedream/api/resources/deployedtriggers/AsyncRawDeployedTriggersClient.javasrc/main/java/com/pipedream/api/resources/deployedtriggers/RawDeployedTriggersClient.javasrc/main/java/com/pipedream/api/resources/filestash/AsyncRawFileStashClient.javasrc/main/java/com/pipedream/api/resources/filestash/RawFileStashClient.javasrc/main/java/com/pipedream/api/resources/oauthtokens/AsyncRawOauthTokensClient.javasrc/main/java/com/pipedream/api/resources/oauthtokens/RawOauthTokensClient.javasrc/main/java/com/pipedream/api/resources/projectenvironment/AsyncRawProjectEnvironmentClient.javasrc/main/java/com/pipedream/api/resources/projectenvironment/RawProjectEnvironmentClient.javasrc/main/java/com/pipedream/api/resources/projects/AsyncProjectsClient.javasrc/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.javasrc/main/java/com/pipedream/api/resources/projects/ProjectsClient.javasrc/main/java/com/pipedream/api/resources/projects/RawProjectsClient.javasrc/main/java/com/pipedream/api/resources/tokens/AsyncRawTokensClient.javasrc/main/java/com/pipedream/api/resources/tokens/RawTokensClient.javasrc/main/java/com/pipedream/api/resources/triggers/AsyncRawTriggersClient.javasrc/main/java/com/pipedream/api/resources/triggers/AsyncTriggersClient.javasrc/main/java/com/pipedream/api/resources/triggers/RawTriggersClient.javasrc/main/java/com/pipedream/api/resources/triggers/TriggersClient.javasrc/main/java/com/pipedream/api/resources/usage/AsyncRawUsageClient.javasrc/main/java/com/pipedream/api/resources/usage/RawUsageClient.javasrc/main/java/com/pipedream/api/resources/users/AsyncRawUsersClient.javasrc/main/java/com/pipedream/api/resources/users/AsyncUsersClient.javasrc/main/java/com/pipedream/api/resources/users/RawUsersClient.javasrc/main/java/com/pipedream/api/resources/users/UsersClient.javasrc/main/java/com/pipedream/api/types/ConfigurablePropAirtableBaseId.javasrc/main/java/com/pipedream/api/types/ConfigurablePropAirtableFieldId.javasrc/main/java/com/pipedream/api/types/ConfigurablePropAirtableTableId.javasrc/main/java/com/pipedream/api/types/ConfigurablePropAirtableViewId.javasrc/main/java/com/pipedream/api/types/ConfigurablePropAlert.javasrc/main/java/com/pipedream/api/types/ConfigurablePropAny.javasrc/main/java/com/pipedream/api/types/ConfigurablePropApp.javasrc/main/java/com/pipedream/api/types/ConfigurablePropApphook.javasrc/main/java/com/pipedream/api/types/ConfigurablePropBase.javasrc/main/java/com/pipedream/api/types/ConfigurablePropBoolean.javasrc/main/java/com/pipedream/api/types/ConfigurablePropDataStore.javasrc/main/java/com/pipedream/api/types/ConfigurablePropDb.javasrc/main/java/com/pipedream/api/types/ConfigurablePropDir.javasrc/main/java/com/pipedream/api/types/ConfigurablePropDiscord.javasrc/main/java/com/pipedream/api/types/ConfigurablePropDiscordChannel.javasrc/main/java/com/pipedream/api/types/ConfigurablePropDiscordChannelArray.javasrc/main/java/com/pipedream/api/types/ConfigurablePropHttp.javasrc/main/java/com/pipedream/api/types/ConfigurablePropHttpRequest.javasrc/main/java/com/pipedream/api/types/ConfigurablePropInteger.javasrc/main/java/com/pipedream/api/types/ConfigurablePropIntegerArray.javasrc/main/java/com/pipedream/api/types/ConfigurablePropObject.javasrc/main/java/com/pipedream/api/types/ConfigurablePropSql.javasrc/main/java/com/pipedream/api/types/ConfigurablePropString.javasrc/main/java/com/pipedream/api/types/ConfigurablePropStringArray.javasrc/main/java/com/pipedream/api/types/ConfigurablePropTimer.javasrc/test/java/com/pipedream/api/OauthTokensWireTest.javasrc/test/java/com/pipedream/api/ProxyClientWireTest.javasrc/test/java/com/pipedream/api/StreamTest.javasrc/test/java/com/pipedream/api/TokensWireTest.javasrc/test/java/com/pipedream/api/testutil/MockResponse.javasrc/test/java/com/pipedream/api/testutil/MockWebServer.javasrc/test/java/com/pipedream/api/testutil/RecordedRequest.java
Comment on lines
+267
to
+309
| public static final class _TokenAuth extends AsyncBaseClientBuilder { | ||
| private final String token; | ||
|
|
||
| _TokenAuth(String token) { | ||
| this.token = token; | ||
| } | ||
|
|
||
| @Override | ||
| protected void setAuthentication(ClientOptions.Builder builder) { | ||
| builder.addHeader("Authorization", "Bearer " + this.token); | ||
| } | ||
| } | ||
|
|
||
| public static final class _CredentialsAuth extends AsyncBaseClientBuilder { | ||
| private final String clientId; | ||
|
|
||
| private final String clientSecret; | ||
|
|
||
| private String scope = null; | ||
|
|
||
| _CredentialsAuth(String clientId, String clientSecret) { | ||
| this.clientId = clientId; | ||
| this.clientSecret = clientSecret; | ||
| } | ||
|
|
||
| public _CredentialsAuth scope(String scope) { | ||
| this.scope = scope; | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public AsyncBaseClient build() { | ||
| validateConfiguration(); | ||
| ClientOptions baseOptions = buildClientOptions(); | ||
| OauthTokensClient authClient = new OauthTokensClient(baseOptions); | ||
| OAuthTokenSupplier oAuthTokenSupplier = | ||
| new OAuthTokenSupplier(this.clientId, this.clientSecret, this.scope, authClient); | ||
| ClientOptions finalOptions = ClientOptions.Builder.from(baseOptions) | ||
| .addHeader("Authorization", oAuthTokenSupplier) | ||
| .build(); | ||
| return new AsyncBaseClient(finalOptions); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/com/pipedream/api/AsyncBaseClientBuilder.java | head -100Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 3865
🏁 Script executed:
wc -l src/main/java/com/pipedream/api/AsyncBaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 138
🏁 Script executed:
# Get the class declaration and fluent method definitions
rg "public class AsyncBaseClientBuilder|public.*\(.*\)\s*\{" src/main/java/com/pipedream/api/AsyncBaseClientBuilder.java -A 2 | head -80Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 1812
🏁 Script executed:
# Look for generic parameters in the class definition
sed -n '1,50p' src/main/java/com/pipedream/api/AsyncBaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1828
🏁 Script executed:
sed -n '267,320p' src/main/java/com/pipedream/api/AsyncBaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1570
🏁 Script executed:
# Check if there are any other nested builder classes to compare against
rg "extends AsyncBaseClientBuilder" src/main/java/com/pipedream/api/AsyncBaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 303
Parameterize the nested auth builders to preserve fluent chaining.
_TokenAuth and _CredentialsAuth currently extend the raw AsyncBaseClientBuilder, so inherited fluent setters return raw AsyncBaseClientBuilder instead of the subtype. This breaks chains like withCredentials(...).projectId(...).scope(...) where the .projectId() call returns the raw parent type lacking the subtype-specific scope() method. Both nested classes should extend AsyncBaseClientBuilder<_TokenAuth> and AsyncBaseClientBuilder<_CredentialsAuth> respectively.
Since this is a Fern-generated file, the fix must be applied upstream in the Fern API definition or generator configuration and incorporated via regeneration.
🤖 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 `@src/main/java/com/pipedream/api/AsyncBaseClientBuilder.java` around lines 267
- 309, The nested auth builder classes _TokenAuth and _CredentialsAuth must be
generic-specialized so fluent chaining returns the subclass type: change their
inheritance from AsyncBaseClientBuilder to AsyncBaseClientBuilder<_TokenAuth>
and AsyncBaseClientBuilder<_CredentialsAuth> respectively so methods like
scope(), projectId(), etc. return the correct subtype; apply this change in the
Fern API definition or generator config and regenerate the file so the generated
classes preserve fluent chaining for those builders.
Comment on lines
+267
to
+309
| public static final class _TokenAuth extends BaseClientBuilder { | ||
| private final String token; | ||
|
|
||
| _TokenAuth(String token) { | ||
| this.token = token; | ||
| } | ||
|
|
||
| @Override | ||
| protected void setAuthentication(ClientOptions.Builder builder) { | ||
| builder.addHeader("Authorization", "Bearer " + this.token); | ||
| } | ||
| } | ||
|
|
||
| public static final class _CredentialsAuth extends BaseClientBuilder { | ||
| private final String clientId; | ||
|
|
||
| private final String clientSecret; | ||
|
|
||
| private String scope = null; | ||
|
|
||
| _CredentialsAuth(String clientId, String clientSecret) { | ||
| this.clientId = clientId; | ||
| this.clientSecret = clientSecret; | ||
| } | ||
|
|
||
| public _CredentialsAuth scope(String scope) { | ||
| this.scope = scope; | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public BaseClient build() { | ||
| validateConfiguration(); | ||
| ClientOptions baseOptions = buildClientOptions(); | ||
| OauthTokensClient authClient = new OauthTokensClient(baseOptions); | ||
| OAuthTokenSupplier oAuthTokenSupplier = | ||
| new OAuthTokenSupplier(this.clientId, this.clientSecret, this.scope, authClient); | ||
| ClientOptions finalOptions = ClientOptions.Builder.from(baseOptions) | ||
| .addHeader("Authorization", oAuthTokenSupplier) | ||
| .build(); | ||
| return new BaseClient(finalOptions); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the complete BaseClientBuilder class definition
cat -n src/main/java/com/pipedream/api/BaseClientBuilder.java | head -100Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 3855
🏁 Script executed:
# Get the total line count to understand the file size
wc -l src/main/java/com/pipedream/api/BaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 133
🏁 Script executed:
# Let's examine the class declaration and see if it uses generics
head -50 src/main/java/com/pipedream/api/BaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1818
🏁 Script executed:
# Now let's look at the nested auth builder declarations (lines 267-309)
sed -n '267,309p' src/main/java/com/pipedream/api/BaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1548
🏁 Script executed:
# Let's also check if there are any test files or usage examples showing the chaining pattern
fd -e java -type f | head -20Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 245
🏁 Script executed:
# Search for usage of these nested builders to understand the fluent chaining pattern
rg "_TokenAuth|_CredentialsAuth" -A 3 -B 1 --max-count=20Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 8285
🏁 Script executed:
# Let's examine the projectId method in BaseClientBuilder to see its signature
rg "projectId" src/main/java/com/pipedream/api/BaseClientBuilder.java -A 2Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 439
🏁 Script executed:
# Check if there are any integration tests or examples that show chaining patterns
find . -name "*Test*.java" -o -name "*Example*.java" | head -10Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 412
🏁 Script executed:
# Let's check the test files to see if there are any examples of fluent chaining
cat src/test/java/com/pipedream/api/TestClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 321
🏁 Script executed:
# Let's also verify the issue by looking at what happens when you call inherited methods
# Check if there's any documentation or CLAUDE.md that mentions this issue
cat CLAUDE.md | head -100Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 6797
🏁 Script executed:
# Let's check the .fernignore file to see what's hand-maintained
cat .fernignoreRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 2529
🏁 Script executed:
# Let's verify the actual return types when calling inherited methods on raw-extended class
# Create a simple test to demonstrate the type issue
cat > /tmp/test_typing.java << 'EOF'
// Pseudo-Java to illustrate the typing issue
class BaseClientBuilder<T extends BaseClientBuilder<T>> {
public T projectId(String projectId) {
return (T) this;
}
}
class _CredentialsAuth extends BaseClientBuilder {
// extends raw BaseClientBuilder, not BaseClientBuilder<_CredentialsAuth>
public _CredentialsAuth scope(String scope) {
return this;
}
}
// Usage:
// BaseClientBuilder builder = new _CredentialsAuth(...);
// builder.projectId("x") returns BaseClientBuilder (raw T), not _CredentialsAuth
// So calling .scope(...) on it would fail
// Correct version would be:
class _CredentialsAuthFixed extends BaseClientBuilder<_CredentialsAuthFixed> {
public _CredentialsAuthFixed scope(String scope) {
return this;
}
}
EOF
cat /tmp/test_typing.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 850
🏁 Script executed:
# Check if there's any test or example showing the chaining pattern with these nested builders
rg "withCredentials.*projectId|withToken.*projectId" -A 5 -B 5Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 56
🏁 Script executed:
# Let's double-check the exact issue by looking at what _CredentialsAuth scope() returns
sed -n '290,300p' src/main/java/com/pipedream/api/BaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 360
🏁 Script executed:
# Verify the inheritance issue by checking AsyncBaseClientBuilder for comparison
grep -A 3 "class _CredentialsAuth extends" src/main/java/com/pipedream/api/AsyncBaseClientBuilder.java | head -5Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 237
🏁 Script executed:
# Verify that AsyncBaseClientBuilder has the same issue and is also hand-maintained
head -20 src/main/java/com/pipedream/api/AsyncBaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 751
🏁 Script executed:
# Check the class declaration for AsyncBaseClientBuilder to confirm it also uses generics
grep "public class AsyncBaseClientBuilder" src/main/java/com/pipedream/api/AsyncBaseClientBuilder.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 149
🏁 Script executed:
# Verify one more time by looking at the CLAUDE.md notes about BaseClientBuilder being protected
grep -A 5 "BaseClientBuilder.java" CLAUDE.md | head -10Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 1790
Parameterize the nested auth builders to preserve fluent chaining.
_TokenAuth and _CredentialsAuth extend the raw BaseClientBuilder, so inherited methods like projectId(), url(), and timeout() return raw BaseClientBuilder instead of the subtype. This breaks valid chains such as withCredentials(...).projectId(...).scope(...). These classes need to extend BaseClientBuilder<_TokenAuth> and BaseClientBuilder<_CredentialsAuth> respectively. The same issue exists in AsyncBaseClientBuilder's nested auth classes.
This file is hand-maintained (listed in .fernignore) and can be fixed locally.
🤖 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 `@src/main/java/com/pipedream/api/BaseClientBuilder.java` around lines 267 -
309, The nested auth builders _TokenAuth and _CredentialsAuth currently extend
the raw BaseClientBuilder which causes inherited fluent methods (projectId(),
url(), timeout(), etc.) to return BaseClientBuilder and break chaining; change
their declarations to extend BaseClientBuilder<_TokenAuth> and
BaseClientBuilder<_CredentialsAuth> respectively (and do the same for the
corresponding nested auth classes in AsyncBaseClientBuilder), ensuring the class
generic parameter matches the concrete subclass so methods like
withCredentials(...).projectId(...).scope(...) continue to return the correct
subtype and preserve fluent chaining.
Comment on lines
191
to
+198
| return new ClientOptions( | ||
| environment, headers, headerSuppliers, httpClient, this.timeout.get(), this.projectId); | ||
| environment, | ||
| headers, | ||
| headerSuppliers, | ||
| httpClient, | ||
| this.timeout.get(), | ||
| this.maxRetries, | ||
| this.projectId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show that Builder.from(...) carries the existing OkHttpClient forward
# while build() only adds RetryInterceptor when no client is already present.
sed -n '154,212p' src/main/java/com/pipedream/api/core/ClientOptions.java
sed -n '1,40p' src/main/java/com/pipedream/api/core/RetryInterceptor.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 3756
🏁 Script executed:
#!/bin/bash
# 1. Confirm this is a Fern-generated file
head -5 src/main/java/com/pipedream/api/core/ClientOptions.java
# 2. Check if there are any other references to maxRetries that might apply the new value
rg "maxRetries" src/main/java/com/pipedream/api/core/ -A 2 -B 2
# 3. Understand how OkHttpClient.newBuilder() handles interceptors
# to confirm if old interceptors are preserved
rg "newBuilder\|addInterceptor" src/main/java/com/pipedream/api/core/ClientOptions.java -B 2 -A 2Repository: PipedreamHQ/pipedream-sdk-java
Length of output: 4891
maxRetries is silently ignored once the builder already has an OkHttpClient.
build() only installs RetryInterceptor in the this.httpClient == null branch, but Builder.from(...) now always copies the existing httpClient from the source ClientOptions. That means Builder.from(existing).maxRetries(5).build() returns maxRetries()==5 while the cloned OkHttpClient retains its old interceptor configuration. The same mismatch occurs when callers set both httpClient(...) and maxRetries(...), since OkHttpClient.newBuilder() preserves all interceptors from the original client.
This is a generated file. Fix upstream in Fern.
🤖 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 `@src/main/java/com/pipedream/api/core/ClientOptions.java` around lines 191 -
198, Builder.from(...) currently copies an existing OkHttpClient but build()
only installs RetryInterceptor when httpClient was null, so maxRetries changes
are ignored; to fix, always obtain an OkHttpClient.Builder (either
this.httpClient.newBuilder() or new OkHttpClient.Builder()) inside
ClientOptions.build(), remove any existing RetryInterceptor instances from its
interceptors, and then add a new RetryInterceptor configured with
this.maxRetries (skipping if maxRetries indicates "no retries"). This ensures
changes made via Builder.maxRetries(...) or Builder.httpClient(...) are
reflected in the resulting OkHttpClient regardless of whether an existing client
was provided.
Comment on lines
+70
to
+76
| public static <T> CompletableFuture<AsyncCustomPager<T>> createAsync( | ||
| Object initialResponse, ClientOptions clientOptions, Object requestOptions) { | ||
| throw new UnsupportedOperationException("AsyncCustomPager must be implemented. " | ||
| + "Please implement this class in core/AsyncCustomPager.java to define your async pagination logic. " | ||
| + "This file has been added to .fernignore and will not be overwritten. " | ||
| + "See the class documentation for implementation examples."); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Async stubs createAsync, getAllItemsAsync, and forEachPageAsync should fail via the returned future, not throw synchronously.
nextPageAsync / previousPageAsync correctly route the UnsupportedOperationException through CompletableFuture.completeExceptionally, but these three methods throw synchronously despite their return type being CompletableFuture<…>. Callers composing with thenApply / handle won't observe the failure as a future completion, which is inconsistent with the rest of this skeleton and may get copied verbatim by implementers.
Suggested alignment
public static <T> CompletableFuture<AsyncCustomPager<T>> createAsync(
Object initialResponse, ClientOptions clientOptions, Object requestOptions) {
- throw new UnsupportedOperationException("AsyncCustomPager must be implemented. "
- + "Please implement this class in core/AsyncCustomPager.java to define your async pagination logic. "
- + "This file has been added to .fernignore and will not be overwritten. "
- + "See the class documentation for implementation examples.");
+ CompletableFuture<AsyncCustomPager<T>> future = new CompletableFuture<>();
+ future.completeExceptionally(new UnsupportedOperationException(
+ "AsyncCustomPager must be implemented. "
+ + "Please implement this class in core/AsyncCustomPager.java to define your async pagination logic. "
+ + "This file has been added to .fernignore and will not be overwritten. "
+ + "See the class documentation for implementation examples."));
+ return future;
}
@@
public CompletableFuture<List<T>> getAllItemsAsync() {
- throw new UnsupportedOperationException("AsyncCustomPager.getAllItemsAsync() must be implemented. "
- + "This method should asynchronously fetch all pages and return all items.");
+ CompletableFuture<List<T>> future = new CompletableFuture<>();
+ future.completeExceptionally(new UnsupportedOperationException(
+ "AsyncCustomPager.getAllItemsAsync() must be implemented. "
+ + "This method should asynchronously fetch all pages and return all items."));
+ return future;
}
@@
public CompletableFuture<Void> forEachPageAsync(
java.util.function.Function<List<T>, CompletionStage<Void>> pageProcessor) {
- throw new UnsupportedOperationException("AsyncCustomPager.forEachPageAsync() must be implemented. "
- + "This method should asynchronously process each page with the given function.");
+ CompletableFuture<Void> future = new CompletableFuture<>();
+ future.completeExceptionally(new UnsupportedOperationException(
+ "AsyncCustomPager.forEachPageAsync() must be implemented. "
+ + "This method should asynchronously process each page with the given function."));
+ return future;
}If this skeleton is generated upstream by Fern, the change should land in the generator template rather than be patched here.
Also applies to: 149-164
🤖 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 `@src/main/java/com/pipedream/api/core/pagination/AsyncCustomPager.java` around
lines 70 - 76, The three async stub methods createAsync, getAllItemsAsync, and
forEachPageAsync currently throw UnsupportedOperationException synchronously;
change them to return a CompletableFuture that is already completed
exceptionally with the same UnsupportedOperationException (use
CompletableFuture.failedFuture or create a new CompletableFuture and call
completeExceptionally) so callers observe the error via the returned future;
mirror the pattern used in nextPageAsync/previousPageAsync to route errors
through the future rather than throwing synchronously.
Comment on lines
+69
to
+74
| public static <T> CustomPager<T> create(Object initialResponse, ClientOptions clientOptions, Object requestOptions) | ||
| throws IOException { | ||
| throw new UnsupportedOperationException("CustomPager must be implemented. " | ||
| + "Please implement this class in core/CustomPager.java to define your pagination logic. " | ||
| + "This file has been added to .fernignore and will not be overwritten. " | ||
| + "See the class documentation for implementation examples."); |
There was a problem hiding this comment.
Point the exception at the real file path.
The message tells users to implement core/CustomPager.java, but this class lives under core/pagination/CustomPager.java. As written, the first runtime failure sends them to the wrong location.
Suggested fix
- throw new UnsupportedOperationException("CustomPager must be implemented. "
- + "Please implement this class in core/CustomPager.java to define your pagination logic. "
+ throw new UnsupportedOperationException("CustomPager must be implemented. "
+ + "Please implement this class in core/pagination/CustomPager.java to define your pagination logic. "
+ "This file has been added to .fernignore and will not be overwritten. "
+ "See the class documentation for implementation examples.");📝 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.
Suggested change
| public static <T> CustomPager<T> create(Object initialResponse, ClientOptions clientOptions, Object requestOptions) | |
| throws IOException { | |
| throw new UnsupportedOperationException("CustomPager must be implemented. " | |
| + "Please implement this class in core/CustomPager.java to define your pagination logic. " | |
| + "This file has been added to .fernignore and will not be overwritten. " | |
| + "See the class documentation for implementation examples."); | |
| public static <T> CustomPager<T> create(Object initialResponse, ClientOptions clientOptions, Object requestOptions) | |
| throws IOException { | |
| throw new UnsupportedOperationException("CustomPager must be implemented. " | |
| "Please implement this class in core/pagination/CustomPager.java to define your pagination logic. " | |
| "This file has been added to .fernignore and will not be overwritten. " | |
| "See the class documentation for implementation examples."); |
🤖 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 `@src/main/java/com/pipedream/api/core/pagination/CustomPager.java` around
lines 69 - 74, Update the UnsupportedOperationException message in the static
factory method CustomPager.create to point to the correct file path
core/pagination/CustomPager.java (instead of core/CustomPager.java); modify the
exception text in the create(Object initialResponse, ClientOptions
clientOptions, Object requestOptions) method of class CustomPager so it
instructs users to implement the class at core/pagination/CustomPager.java and
retains the rest of the guidance about .fernignore and implementation examples.
Comment on lines
+56
to
+61
| /** | ||
| * List the projects that are available to the authenticated Connect client | ||
| */ | ||
| public CompletableFuture<BaseClientHttpResponse<SyncPagingIterable<Project>>> list(RequestOptions requestOptions) { | ||
| return list(ProjectsListRequest.builder().build(), requestOptions); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and inspect the file
fd -t f "AsyncRawProjectsClient.java" --type fRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 153
🏁 Script executed:
# Check file size and read the relevant sections
wc -l src/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 157
🏁 Script executed:
# Read lines around 56-61 to see the list() method overloads
sed -n '40,75p' src/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1480
🏁 Script executed:
# Read lines around 370-375 to see the update() method overloads
sed -n '355,395p' src/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1629
🏁 Script executed:
# Check if file is Fern-generated (look for generation markers)
head -30 src/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1462
Avoid overloads that make null call sites ambiguous.
The new list(RequestOptions requestOptions) and update(String projectId, RequestOptions requestOptions) overloads collide with existing request-object overloads. Calls like list(null) and update(projectId, null) are now ambiguous between the RequestOptions and request-object variants, causing compilation errors. Because this is generated surface area, the overload shape must be corrected upstream in the Fern API definition or generator template rather than via manual edits.
Also applies to: 370-375
🤖 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
`@src/main/java/com/pipedream/api/resources/projects/AsyncRawProjectsClient.java`
around lines 56 - 61, The new overloads list(RequestOptions) and update(String,
RequestOptions) in AsyncRawProjectsClient collide with existing request-object
overloads and create ambiguous null call-sites; fix this in the Fern
API/generator template (not by hand) so it does not emit overloads that only
differ by a nullable RequestOptions parameter—either remove these overloads,
give the RequestOptions variants distinct names (e.g., listWithOptions /
updateWithOptions), or alter the signature shape (wrap RequestOptions in a
distinct type or make it a non-ambiguous last vararg) so calls like list(null)
and update(projectId, null) are unambiguous; update the generator rule that
produces the list(RequestOptions) and update(String, RequestOptions) signatures
in AsyncRawProjectsClient to follow one of these non-ambiguous patterns.
Comment on lines
+51
to
+56
| /** | ||
| * List the projects that are available to the authenticated Connect client | ||
| */ | ||
| public BaseClientHttpResponse<SyncPagingIterable<Project>> list(RequestOptions requestOptions) { | ||
| return list(ProjectsListRequest.builder().build(), requestOptions); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the key sections
wc -l src/main/java/com/pipedream/api/resources/projects/RawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 152
🏁 Script executed:
# Check for Fern auto-generation marker
head -20 src/main/java/com/pipedream/api/resources/projects/RawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 996
🏁 Script executed:
# Read lines around 51-56 to see the list() overloads
sed -n '45,75p' src/main/java/com/pipedream/api/resources/projects/RawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1403
🏁 Script executed:
# Read lines around 299-300 to see the update() overloads
sed -n '290,320p' src/main/java/com/pipedream/api/resources/projects/RawProjectsClient.javaRepository: PipedreamHQ/pipedream-sdk-java
Length of output: 1259
Null-ambiguous overloads break source compatibility.
Adding list(RequestOptions) alongside list(ProjectsListRequest) and update(String, RequestOptions) alongside update(String, UpdateProjectOpts) causes calls like list(null) and update(projectId, null) to fail compilation. Java cannot resolve which overload should be selected when both parameters are reference types that null satisfies. This is a source-compatibility break in the public API and must be addressed in the Fern generator or API definition upstream—not patched locally in generated code.
Also applies to: 299-300
🤖 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 `@src/main/java/com/pipedream/api/resources/projects/RawProjectsClient.java`
around lines 51 - 56, The new null-ambiguous overloads break source
compatibility: remove or revert the newly added overloads that accept only
RequestOptions (list(RequestOptions)) and only RequestOptions for update
(update(String, RequestOptions)), and instead ensure the Fern generator / API
definition upstream produces a single unambiguous method signature (keep
list(ProjectsListRequest) and update(String, UpdateProjectOpts) or give the
alternate methods distinct names). Locate the generated methods
list(RequestOptions), list(ProjectsListRequest), update(String, RequestOptions),
and update(String, UpdateProjectOpts) and remove or rename the ambiguous
RequestOptions-only overloads so calls like list(null) and update(id, null) no
longer cause compilation ambiguity; fix must be applied in the generator/spec
rather than ad-hoc in generated code.
Comment on lines
+25
to
+27
| client = PipedreamClient.builder() | ||
| .url(server.url("/").toString()) | ||
| .build(); |
There was a problem hiding this comment.
Wire test may trigger OAuth flow if PIPEDREAM_CLIENT_ID/PIPEDREAM_CLIENT_SECRET are set in the environment.
PipedreamClient.builder() pre-populates clientId/clientSecret from env vars, so on a developer machine or CI with those secrets exported, this test will hit a real OAuth endpoint instead of (or before) the mock server, leading to flaky/incorrect results. Pass explicit nulls to neutralize env-var defaults:
🛡️ Proposed fix
client = PipedreamClient.builder()
.url(server.url("/").toString())
+ .clientId(null)
+ .clientSecret(null)
.build();Since this is a Fern-generated wire test, the change must be made upstream in the Fern generator/template (or this file added to .fernignore and hand-maintained). As per coding guidelines: "Be aware that builder() reads PIPEDREAM_CLIENT_ID and PIPEDREAM_CLIENT_SECRET from the environment, so unset them locally or pass .clientId(null).clientSecret(null) to prevent OAuth flow."
📝 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.
Suggested change
| client = PipedreamClient.builder() | |
| .url(server.url("/").toString()) | |
| .build(); | |
| client = PipedreamClient.builder() | |
| .url(server.url("/").toString()) | |
| .clientId(null) | |
| .clientSecret(null) | |
| .build(); |
🤖 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 `@src/test/java/com/pipedream/api/OauthTokensWireTest.java` around lines 25 -
27, The test currently calls PipedreamClient.builder().url(...).build(), but
builder() pulls PIPEDREAM_CLIENT_ID/PIPEDREAM_CLIENT_SECRET from the environment
and can trigger a real OAuth flow; update the builder chain for the test to
neutralize those env defaults by explicitly setting
.clientId(null).clientSecret(null) before calling build() (i.e., use
PipedreamClient.builder().url(...).clientId(null).clientSecret(null).build());
because this is a Fern-generated test, also make the corresponding change in the
Fern generator/template (or add this test to .fernignore) so regenerated code
preserves the neutralized credentials.
Comment on lines
36
to
45
| // Explicitly set credentials to null to avoid OAuth token fetching | ||
| // (environment variables might be set, which would trigger network calls) | ||
| client = BaseClient.builder() | ||
| client = PipedreamClient.builder() | ||
| .url(baseUrl) | ||
| .projectId("test-project") | ||
| .clientId(null) | ||
| .clientSecret(null) | ||
| .build(); | ||
| asyncClient = AsyncBaseClient.builder() | ||
| asyncClient = AsyncPipedreamClient.builder() | ||
| .url(baseUrl) | ||
| .projectId("test-project") | ||
| .clientId(null) | ||
| .clientSecret(null) | ||
| .build(); |
There was a problem hiding this comment.
Comment claims credentials are nulled, but the builder calls don't actually null them — wire test can trigger OAuth against the mock server.
The comment on lines 36–37 says credentials are explicitly nulled to avoid OAuth token fetching, but PipedreamClient.builder() / AsyncPipedreamClient.builder() read PIPEDREAM_CLIENT_ID and PIPEDREAM_CLIENT_SECRET from the environment unless you pass .clientId(null).clientSecret(null). With those env vars set on a developer machine or CI, every test request will first try an OAuth token fetch (against the mock server URL), making these tests order/environment-dependent.
Suggested fix
client = PipedreamClient.builder()
+ .clientId(null)
+ .clientSecret(null)
.url(baseUrl)
.projectId("test-project")
.build();
asyncClient = AsyncPipedreamClient.builder()
+ .clientId(null)
+ .clientSecret(null)
.url(baseUrl)
.projectId("test-project")
.build();As per coding guidelines: "Wire tests should construct clients via PipedreamClient.builder().url(server.url("/").toString()).build() … Be aware that builder() reads PIPEDREAM_CLIENT_ID and PIPEDREAM_CLIENT_SECRET from the environment, so unset them locally or pass .clientId(null).clientSecret(null) to prevent OAuth flow."
📝 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.
Suggested change
| // Explicitly set credentials to null to avoid OAuth token fetching | |
| // (environment variables might be set, which would trigger network calls) | |
| client = BaseClient.builder() | |
| client = PipedreamClient.builder() | |
| .url(baseUrl) | |
| .projectId("test-project") | |
| .clientId(null) | |
| .clientSecret(null) | |
| .build(); | |
| asyncClient = AsyncBaseClient.builder() | |
| asyncClient = AsyncPipedreamClient.builder() | |
| .url(baseUrl) | |
| .projectId("test-project") | |
| .clientId(null) | |
| .clientSecret(null) | |
| .build(); | |
| // Explicitly set credentials to null to avoid OAuth token fetching | |
| // (environment variables might be set, which would trigger network calls) | |
| client = PipedreamClient.builder() | |
| .clientId(null) | |
| .clientSecret(null) | |
| .url(baseUrl) | |
| .projectId("test-project") | |
| .build(); | |
| asyncClient = AsyncPipedreamClient.builder() | |
| .clientId(null) | |
| .clientSecret(null) | |
| .url(baseUrl) | |
| .projectId("test-project") | |
| .build(); |
🤖 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 `@src/test/java/com/pipedream/api/ProxyClientWireTest.java` around lines 36 -
45, The comment is wrong: the builders read
PIPEDREAM_CLIENT_ID/PIPEDREAM_CLIENT_SECRET from the environment and will
trigger OAuth; fix both client constructions (PipedreamClient.builder() and
AsyncPipedreamClient.builder()) by explicitly disabling env credentials via
.clientId(null).clientSecret(null) before
.url(baseUrl).projectId("test-project").build() so the wire tests do not attempt
token fetching.
Comment on lines
+70
to
+90
| MockResponse response = responses.poll(); | ||
| if (response == null) { | ||
| exchange.sendResponseHeaders(503, -1); | ||
| exchange.close(); | ||
| return; | ||
| } | ||
|
|
||
| for (Map.Entry<String, String> header : response.headers.entrySet()) { | ||
| exchange.getResponseHeaders().add(header.getKey(), header.getValue()); | ||
| } | ||
|
|
||
| if (response.body.length == 0) { | ||
| exchange.sendResponseHeaders(response.code, -1); | ||
| } else { | ||
| exchange.sendResponseHeaders(response.code, response.body.length); | ||
| try (OutputStream out = exchange.getResponseBody()) { | ||
| out.write(response.body); | ||
| } | ||
| } | ||
| exchange.close(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Minor: handle() will leak the exchange if a downstream call throws.
Between responses.poll() and the final exchange.close() (line 89), any exception from sendResponseHeaders / out.write propagates out of handle without closing the exchange — the JDK HttpServer worker logs and moves on, but the connection is left to GC-finalize. A try { … } finally { exchange.close(); } around the body-write block would make cleanup deterministic. Low priority since this only matters on test-time failures.
Suggested cleanup
- private void handle(HttpExchange exchange) throws IOException {
- byte[] requestBody = readAllBytes(exchange.getRequestBody());
- requests.add(new RecordedRequest(
- exchange.getRequestMethod(),
- exchange.getRequestURI().toString(),
- requestBody));
-
- MockResponse response = responses.poll();
- if (response == null) {
- exchange.sendResponseHeaders(503, -1);
- exchange.close();
- return;
- }
-
- for (Map.Entry<String, String> header : response.headers.entrySet()) {
- exchange.getResponseHeaders().add(header.getKey(), header.getValue());
- }
-
- if (response.body.length == 0) {
- exchange.sendResponseHeaders(response.code, -1);
- } else {
- exchange.sendResponseHeaders(response.code, response.body.length);
- try (OutputStream out = exchange.getResponseBody()) {
- out.write(response.body);
- }
- }
- exchange.close();
- }
+ private void handle(HttpExchange exchange) throws IOException {
+ try {
+ byte[] requestBody = readAllBytes(exchange.getRequestBody());
+ requests.add(new RecordedRequest(
+ exchange.getRequestMethod(),
+ exchange.getRequestURI().toString(),
+ requestBody));
+
+ MockResponse response = responses.poll();
+ if (response == null) {
+ exchange.sendResponseHeaders(503, -1);
+ return;
+ }
+
+ for (Map.Entry<String, String> header : response.headers.entrySet()) {
+ exchange.getResponseHeaders().add(header.getKey(), header.getValue());
+ }
+
+ if (response.body.length == 0) {
+ exchange.sendResponseHeaders(response.code, -1);
+ } else {
+ exchange.sendResponseHeaders(response.code, response.body.length);
+ try (OutputStream out = exchange.getResponseBody()) {
+ out.write(response.body);
+ }
+ }
+ } finally {
+ exchange.close();
+ }
+ }📝 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.
Suggested change
| MockResponse response = responses.poll(); | |
| if (response == null) { | |
| exchange.sendResponseHeaders(503, -1); | |
| exchange.close(); | |
| return; | |
| } | |
| for (Map.Entry<String, String> header : response.headers.entrySet()) { | |
| exchange.getResponseHeaders().add(header.getKey(), header.getValue()); | |
| } | |
| if (response.body.length == 0) { | |
| exchange.sendResponseHeaders(response.code, -1); | |
| } else { | |
| exchange.sendResponseHeaders(response.code, response.body.length); | |
| try (OutputStream out = exchange.getResponseBody()) { | |
| out.write(response.body); | |
| } | |
| } | |
| exchange.close(); | |
| } | |
| private void handle(HttpExchange exchange) throws IOException { | |
| try { | |
| byte[] requestBody = readAllBytes(exchange.getRequestBody()); | |
| requests.add(new RecordedRequest( | |
| exchange.getRequestMethod(), | |
| exchange.getRequestURI().toString(), | |
| requestBody)); | |
| MockResponse response = responses.poll(); | |
| if (response == null) { | |
| exchange.sendResponseHeaders(503, -1); | |
| return; | |
| } | |
| for (Map.Entry<String, String> header : response.headers.entrySet()) { | |
| exchange.getResponseHeaders().add(header.getKey(), header.getValue()); | |
| } | |
| if (response.body.length == 0) { | |
| exchange.sendResponseHeaders(response.code, -1); | |
| } else { | |
| exchange.sendResponseHeaders(response.code, response.body.length); | |
| try (OutputStream out = exchange.getResponseBody()) { | |
| out.write(response.body); | |
| } | |
| } | |
| } finally { | |
| exchange.close(); | |
| } | |
| } |
🤖 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 `@src/test/java/com/pipedream/api/testutil/MockWebServer.java` around lines 70
- 90, The handle() method in MockWebServer can leak the HttpExchange if
sendResponseHeaders or writing the response body throws; wrap the whole response
handling logic (from after responses.poll() up to the current exchange.close())
in a try { ... } finally { exchange.close(); } so exchange.close() always runs,
and ensure any early returns (e.g. when response == null) still close the
exchange by moving that null-check response handling into the same try/finally
or closing before returning; update references in MockWebServer.handle()
accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR regenerates code to match the latest API Definition.
Summary by CodeRabbit
New Features
withToken()andwithCredentials()for streamlined client setup.withRawResponse().Bug Fixes
Documentation