Codex-generated pull request#3
Codex-generated pull request#3matthewdavidian wants to merge 1 commit intocodex/plan-integration-for-docupipe-apifrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dc3472797
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| If (cfg.sApiKey<>'') Set psApiKey to cfg.sApiKey | ||
| If (cfg.sBaseUrl<>'') Set psBaseUrl to cfg.sBaseUrl | ||
| If (cfg.iTimeoutSeconds>0) Set piTimeoutSeconds to cfg.iTimeoutSeconds | ||
| If (cfg.iRetryCount>=0) Set piRetryCount to cfg.iRetryCount |
There was a problem hiding this comment.
Configure applies cfg.iRetryCount whenever it is >= 0, but tDocuPipeConfig integers default to 0, so callers that only populate sApiKey unintentionally override the class default of one retry with zero retries. In that common path, transient transport failures, 429s, and 5xxs stop retrying even though the object is initialized with retry support.
Useful? React with 👍 / 👎.
|
|
||
| Send SetMemberValue of hoReq "contentBase64" jsonTypeString (UCharArrayToString(Attachment.uBase64File)) | ||
| Send SetMemberValue of hoReq "mimeType" jsonTypeString Attachment.sMimeType | ||
| Send SetMemberValue of hoReq "filename" jsonTypeString sFilePath |
There was a problem hiding this comment.
The request payload sets "filename" to sFilePath, which commonly contains an absolute local path; this leaks local directory/user information to the external API and can break integrations that validate filename tokens without path separators. The field should be derived from the file basename, not the full path.
Useful? React with 👍 / 👎.
| Set peTransferFlags to ifSecure | ||
| Set piRemotePort to rpHttpSSL |
There was a problem hiding this comment.
The client strips both https:// and http:// from psBaseUrl, but transport is always forced to ifSecure/rpHttpSSL and never updated from the configured URL. If a caller provides an http:// base URL (for local/staging/proxy endpoints), requests will still attempt TLS on port 443 and fail to connect.
Useful? React with 👍 / 👎.
1 participant
Codex generated this pull request, but encountered an unexpected error after generation. This is a placeholder PR message.
Codex Task