feat: add httpRequest and wait runtime helpers [APPS-35694] - #695
feat: add httpRequest and wait runtime helpers [APPS-35694]#695anil-uipath wants to merge 2 commits into
Conversation
|
|
Two findings this run. ResolvedRetryOptions leaks into the public barrel (src/models/common/http.types.ts line 120) - marked internal but re-exported via the common index barrel into src/index.ts. Should move to http.internal-types.ts. Also, wrong type annotation on the resilience test fixture (lines 102-108) - Required includes the deprecated retryDelay and useExponentialBackoff fields; the fixture should be typed as ResolvedRetryOptions to match what computeBackoffDelay actually accepts. Everything else looks solid - the retry engine, body handling, timeout/cancellation wiring, backward-compat deprecated-field mapping, and test coverage are all well done. |
83fb89a to
fa41df3
Compare
| // `QueryParams` permits null/undefined; serializing them would send the literal | ||
| // strings "null"/"undefined" as values, which is never what a caller meant. |
There was a problem hiding this comment.
The comment says QueryParams permits null/undefined, but the type definition is Record<string, string | number | boolean | Array<...>> — neither null nor undefined is in the union. TypeScript callers cannot pass them without a type error; the check is a runtime safeguard for untyped JS callers or as-casted values.
| // `QueryParams` permits null/undefined; serializing them would send the literal | |
| // strings "null"/"undefined" as values, which is never what a caller meant. | |
| // Defensive runtime guard: callers not going through TypeScript (or using `as any`) may | |
| // supply null/undefined; serializing them would send the literal strings "null"/"undefined". |
|
One new finding this run — comment inaccuracy in src/utils/http/params.ts lines 54-55: claims QueryParams permits null/undefined, but the type definition (Record<string, string | number | boolean | Array<...>>) excludes both. The defensive check itself is correct; the comment's stated reason is wrong. Suggestion posted inline. Everything else looks solid: ResolvedRetryOptions correctly in http.internal-types.ts (previous finding resolved ✓), retry/backoff logic correct, backward-compat deprecated-field mapping working, timeout/cancellation wiring sound, test coverage thorough. |
fa41df3 to
0702ea5
Compare
| // Target URLs | ||
| URL: 'https://api.example.com/v1/orders', | ||
| URL_WITH_QUERY: 'https://api.example.com/v1/orders?page=2', | ||
| REDIRECTED_URL: 'https://api.example.com/v1/orders/final', |
There was a problem hiding this comment.
REDIRECTED_URL is defined here but not referenced in any of the four test files that import HTTP_TEST_CONSTANTS (http-request.test.ts, retry-policy.test.ts, api-client.test.ts, http-request.integration.test.ts). Convention: "NEVER leave unused code." It was probably added in anticipation of a unit test that verifies response.url after a redirect, but that test was never written — and convention also says don't design for hypothetical future requirements.
| REDIRECTED_URL: 'https://api.example.com/v1/orders/final', |
Remove the line, or add the corresponding test that exercises it.
|
One new finding this run — |
An API Workflow converted to a coded function can only reach this SDK,
so
anything its HTTP activity could express has to be expressible here.
Built
generic rather than for that one case: no workflow-specific limits, no
workflow-specific field names, and the same options are meant to work on
SDK
service methods later.
Adds `httpRequest(url, init?)`, a fetch convenience for arbitrary URLs.
It sends
no UiPath auth and no tracing headers, since it targets third-party
hosts. Two
contracts differ from the rest of the SDK: a status the server returned
is never
an exception — 4xx and 5xx resolve with `ok: false` — and only a request
that
never produced a response throws, as a NetworkError. The response is a
snapshot
(`status`, `statusText`, `ok`, `headers`, `data`, `url`), so the body is
already
parsed; `responseType` selects json/text/blob/arraybuffer/stream.
`retry` on the request carries the robustness knobs, named after the
HTTP client
ecosystem rather than the workflow config: maxRetries,
initialRetryDelayMs,
backoffStrategy ('constant' | 'linear' | 'exponential'), backoffFactor,
backoffMaxDelayMs, retryableStatusCodes, retryMethods,
retryNetworkErrors,
respectRetryAfter, maxRetryAfterMs. Delays match the API Workflow
runtime's
HttpRetryPolicy attempt-for-attempt across every strategy, delay, cap
and
multiplier combination.
Retrying defaults to the idempotent methods of RFC 9110 (GET, HEAD, PUT,
DELETE,
OPTIONS), matching got, ky and axios-retry. POST and PATCH are excluded
unless
opted into `retryMethods`, because a replayed POST can create the same
resource
twice. A `Retry-After` header is honoured as the server sent it rather
than
squeezed under the backoff cap — `maxRetryAfterMs` bounds it separately,
as in
got and ky. A ReadableStream body disables retrying: it is consumed by
the first
attempt and cannot be replayed.
`timeoutMs` sits on the request, not inside `retry`, because it bounds a
single
attempt whether or not retrying is enabled — matching got, ky, and the
workflow
runtime, which all keep the two side by side.
Adds `wait(durationMs)` for pausing between calls.
The retry engine is shared with ApiClient through `RequestSpec.retry`,
so a later
change can extend retrying to service methods. Its defaults there set
maxRetries
to 0, so every existing SDK call keeps its single-attempt behaviour — a
test
asserts exactly one fetch on a 500 with no options supplied.
Housekeeping: `RequestSpec` gains a working `timeoutMs`, replacing the
`timeoutOptions` field that was declared at repo init and never read, so
service
calls can be given a deadline for the first time; the `TimeoutOptions`
type stays
as deprecated since the name is publicly exported. Array-param
serialization
moves out of ApiClient into `toSearchParams`, shared by both HTTP paths.
No OAuth scope entry is needed — the helper is unauthenticated. No
mkdocs nav or
subpath export — it is not a service.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0702ea5 to
eaddabb
Compare
| : computeBackoffDelay(attempt, settings); | ||
|
|
||
| // The discarded response still holds its connection open until the body is released | ||
| response?.body?.cancel().catch(() => {}); |
There was a problem hiding this comment.
Convention: silent catches must emit a console.warn — even for best-effort cleanup operations — so failures remain observable in the runtime console rather than disappearing silently.
| response?.body?.cancel().catch(() => {}); | |
| response?.body?.cancel().catch((error) => { console.warn('Failed to cancel discarded response body:', error); }); |
If the body is already consumed or locked, the error is benign, but the convention doesn't distinguish by severity: a silent .catch() hides the failure from developers who might be debugging connection-pool exhaustion or unexpected body-locking issues.
|
One new finding this run — silent catch without Everything else looks solid: |
| ``` | ||
| ## The `httpRequest` Helper | ||
|
|
There was a problem hiding this comment.
why do we need to add these helpers in error handling doc?
these would end up here: https://uipath.github.io/uipath-typescript/error-handling/
There was a problem hiding this comment.
Agreed — moved to docs/helpers.md ("Helper Methods"), with a nav entry.
| /** | ||
| * Base delay between retries in milliseconds. | ||
| * | ||
| * @deprecated Use {@link RetryOptions.initialDelayMs}. Still honoured when |
There was a problem hiding this comment.
why deprecated? nothing has been released.
There was a problem hiding this comment.
The fields retryDelay and useExponentialBackoff are already in public interface request-spec.ts from the init commit. hence deprecating it
uipath-typescript/src/models/common/request-spec.ts
Lines 26 to 31 in f1d6839
There was a problem hiding this comment.
They did ship — at tag 1.6.2, request-spec.ts has RetryOptions (26), TimeoutOptions (40), RequestSpec.timeoutOptions (94) and RequestSpec.retryOptions (97), all exported publicly via models/common → src/index.ts.
They were never wired to anything, so they did nothing at runtime, but the names are importable — removing them breaks anyone referencing the types. The release policy also blocks it both ways: a patch must stay backward compatible, and a minor requires a deprecation cycle these never had.
If you know there are no external consumers yet, dropping them outright is cleaner and I will do that — just say.
| export interface HttpResponse { | ||
| /** Response status code. */ | ||
| status: number; |
There was a problem hiding this comment.
we dont need type? https://developer.mozilla.org/en-US/docs/Web/API/Response/type
There was a problem hiding this comment.
Left out on purpose. Three of types five values are unreachable here — opaque needs mode: 'no-cors', opaqueredirect needs redirect: 'manual' (we expose neither), and error only comes from a constructed Response.error(), never a resolved fetch. That leaves basic/cors, which is constant in Node.
Added redirected instead — it is the only tell when an API silently redirects to an HTML login page and returns 200.
| /** | ||
| * Request timeout options. | ||
| * | ||
| * @deprecated Use {@link RequestSpec.timeoutMs}. `timeoutOptions.timeout` is still honoured |
There was a problem hiding this comment.
Same as the RetryOptions thread — TimeoutOptions and RequestSpec.timeoutOptions are both in the 1.6.2 tag and publicly exported, so removing them is a breaking change for anyone referencing the names.
TimeoutOptions it had 2 fields
abortOnTimeoutflag is unimplementable with fetch. because for fetch, abort automatically happens on timeout. so removed ittimeout- have implemented this duration field now.
And since it ends up with only 1 field timeout, simplified it for user from the previous
await httpRequest(url, {
body: {},
headers: {},
timeoutOptions: {
timeout: 1000
}
})
to
await httpRequest(url, {
body: {},
headers: {},
timeoutMs: 1000
})
There was a problem hiding this comment.
i dont think we need new file for this
There was a problem hiding this comment.
Fair. Would src/utils/timing.ts work instead? A file named after its only export looks unjustified, but one named after a category does not, and it gives the next small time helper somewhere to land.
I would avoid folding it into http-request.ts — wait is public and is not HTTP, and fetch-with-retry.ts imports it, so that would create a cycle.
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
1948122 to
50ea267
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
50ea267 to
af797d1
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
Concise comments throughout: http.types.ts, http-request.ts, retry-policy.ts and fetch-with-retry.ts all had JSDoc and inline comments that explained more than the reader needs. Trimmed to the non-obvious part in each case. Moves the httpRequest guide out of error-handling.md into its own docs/http-requests.md page with an mkdocs nav entry, leaving a short pointer behind. The helper is a feature, not an error-handling topic, and it had grown to dominate a page about error types. Adds `redirected` to HttpResponse. `Response.type` was raised as a possible addition but three of its five values are unreachable here — `opaque` needs `mode: 'no-cors'`, `opaqueredirect` needs `redirect: 'manual'`, and neither is exposed, while `error` only comes from a constructed `Response.error()`. That leaves basic/cors, which is constant in Node. `redirected` is the property that actually varies, and it is the only signal a caller gets when an API silently redirects to an HTML login page and returns 200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
af797d1 to
8035a09
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
Summary
Adds two runtime helpers to the SDK root export:
httpRequest(url, init?)— afetchconvenience for calling any URL, with configurable retry, backoff, and per-attempt timeout.wait(durationMs)— pause for a duration.Both are standalone functions, not services: they take no
UiPathinstance, send no UiPath authentication, and add no UiPath headers.Motivation
Code that runs as a coded function can only reach this SDK. Calling a third-party API from there previously meant hand-rolling
fetchplus a retry loop in every project.httpRequestmakes that a supported, tested part of the SDK, with the robustness controls — retry count, backoff shape, delay caps, timeouts — expressed as options rather than reimplemented each time.The design is intentionally general rather than shaped around any single caller: no product-specific limits, no product-specific field names, and the same options object is intended to work on SDK service methods in a follow-up.
Behaviour
Two contracts differ from the SDK's service methods, both because this helper proxies arbitrary third-party calls:
A status the server returned is never an exception. A 404 or 500 resolves with
ok: false, so callers can read the response body — which on an error is usually where the vendor's diagnostic lives. Only a request that never produced a response throws, as aNetworkError. (An explicitresponseType: 'json'with an unparseable body raises aServerError.)No authentication or tracing headers are attached. The target is a third-party host, so forwarding
traceparentwould leak internal correlation IDs outside UiPath.The response is a snapshot —
status,statusText,ok,headers,data,url— so the body is already read.responseTypeselectsjson/text/blob/arraybuffer/stream; without it, a JSON content type is parsed and anything else is returned as text.dataisunknownThe body is whatever the server sent: the expected shape on a success, an error payload on a 4xx or 5xx, and nothing at all on a 204. A generic
data: Twould be accurate on only one of those paths, and the error path is the one this helper exists to make readable. Callers assert or validate the shape after checkingok.Retry
retryacceptsmaxRetries,initialDelayMs,backoffStrategy('constant' | 'linear' | 'exponential'),backoffFactor,backoffMaxDelayMs,retryableStatusCodes,retryMethods,retryNetworkErrors,respectRetryAfter, andmaxRetryAfterMs.Defaults follow established HTTP client behaviour:
GET,HEAD,PUT,DELETE,OPTIONS.POSTandPATCHare excluded unless opted intoretryMethods, since a replayedPOSTcan create the same resource twice.408,429,500,502,503,504) and transport failures;retryNetworkErrors: falselimits it to statuses only.Retry-Afterheader as sent, rather than capping it with the backoff limit.maxRetryAfterMsbounds it separately and is unbounded by default.ReadableStreambody — it is consumed by the first attempt and cannot be replayed — or a request cancelled throughsignal.timeoutMssits on the request rather than insideretry, because it bounds a single attempt whether or not retrying is enabled. Each retry starts a fresh timeout.Effect on existing SDK calls
None. The retry engine is shared with
ApiClientthroughRequestSpec, so retrying can be extended to service methods later, but its defaults there setmaxRetries: 0. Every existing call keeps its single-attempt behaviour, and a test asserts exactly onefetchon a 500 when no options are supplied.Service methods keep their own response types and continue to throw on non-2xx.
RequestSpecalso gains a workingtimeoutMs, replacing atimeoutOptionsfield that was declared but never read — so service requests can carry a deadline for the first time.Backward compatibility
RetryOptions,TimeoutOptions,RequestSpec.retryOptionsandRequestSpec.timeoutOptionsare all publicly exported and shipped in 1.6.2, so they continue to work:retryDelayanduseExponentialBackoffremain onRetryOptionsas@deprecated, mapping ontoinitialDelayMsandbackoffStrategyrespectively. Each applies only when its replacement is absent.retryOptionsandtimeoutOptionsremain onRequestSpecas@deprecated, honoured only whenretry/timeoutMsare not supplied.abortOnTimeoutgets no mapping and is documented as having no effect: withfetch, a timeout is an abort.This keeps the change a patch release under the release policy, which requires a deprecation cycle before removal.
Testing
typecheck,lint, unit tests (2558),build, anddocs:validateall pass.tests/unit/utils/http/resilience.test.ts— backoff per strategy, delay caps,Retry-Afterparsing, defaults resolution, deprecated-field mapping and precedencetests/unit/utils/http/http-request.test.ts— non-2xx resolution, retry eligibility per method, stream bodies, network-error toggle, timeouts, cancellation, body parsing, query parameterstests/unit/utils/wait.test.tstests/unit/core/http/api-client.test.ts— the retry-off-by-default contract, per-request opt-in, per-request timeout, deprecated-field fallbackstests/integration/shared/http/http-request.integration.test.ts— live endpoints, including an unauthorised response arriving as data rather than an exceptionNotes
docs/oauth-scopes.mdentry — the helper is unauthenticated.mkdocs.ymlnav entry or subpath export — it is not a service.jitteris not included. Most reference implementations offer it but default it off; it can be added later as an optional field without a breaking change.🤖 Generated with Claude Code