Fix OAuth config check for undefined values #111
Conversation
Updated the OAuth configuration check to ensure 'oauth_con_id' is not undefined before proceeding. This prevents potential issues when 'oauth_con_id' is missing from the config data and makes the first check true if it is undefined
samme-abdul
requested review from
alaa-eddine,
alaa-eddine-k and
zubairirfan96
WalkthroughTightened the OAuth activation condition by guarding against undefined oauth_con_id and changed how OAuth headers are appended to reqConfig.headers using concat instead of a spread-merge. No exported/public APIs were altered. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant APICall
participant OAuthService as OAuthService
participant HTTP as HTTP Client
Caller->>APICall: build request config
alt oauthService present AND oauth_con_id defined/non-empty/non-"None"
APICall->>OAuthService: getOAuthHeaders(oauth_con_id)
OAuthService-->>APICall: oauthHeaders
APICall->>APICall: reqConfig.headers = headers.concat(oauthHeaders)
else No OAuth headers
note over APICall: Skip OAuth header attachment
end
APICall->>HTTP: send(reqConfig)
HTTP-->>APICall: response
APICall-->>Caller: response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/core/src/Components/APICall/APICall.class.ts (3)
118-123: Fix optional-chaining bug and make checks consistent for oauthService.
config.data.oauthServiceon the RHS uses no optional chaining; this can throw whenconfigorconfig.datais undefined. Also add the missingundefinedguard foroauthServiceto mirroroauth_con_id.- if ((config?.data?.oauth_con_id !== undefined && config?.data?.oauth_con_id !== '' && config?.data?.oauth_con_id !== 'None') || (config?.data?.oauthService !== '' && config.data.oauthService !== 'None')) { + if ( + (config?.data?.oauth_con_id !== undefined && + config?.data?.oauth_con_id !== '' && + config?.data?.oauth_con_id !== 'None') || + (config?.data?.oauthService !== undefined && + config?.data?.oauthService !== '' && + config?.data?.oauthService !== 'None') + ) {
100-101: Headers built with Array.concat: Axios expects an object, not an array.
reqConfig.headersmust be a plain object or AxiosHeaders. Using.concatrisks a runtime error (.concat is not a function) or malformed headers.- reqConfig.headers = (await parseHeaders(input, config, agent)).concat({ ...headers }); + reqConfig.headers = { + ...(await parseHeaders(input, config, agent)), + ...(headers || {}), + };
121-123: Do not append OAuth headers with Array.concat; merge objects instead.Same issue as above; merge into the existing headers object.
- //reqConfig.headers = { ...reqConfig.headers, ...oauthHeaders }; - reqConfig.headers = reqConfig.headers.concat({ ...oauthHeaders }); + reqConfig.headers = { ...(reqConfig.headers || {}), ...oauthHeaders };
🧹 Nitpick comments (1)
packages/core/src/Components/APICall/APICall.class.ts (1)
117-123: Optional: centralize “provided value” checks and normalize sentinel values.Reduce repetition and avoid hard-coding 'None' checks everywhere.
Example (within
processbefore the if):const isProvided = (v?: string) => { const s = (v ?? '').trim(); return s !== '' && s.toLowerCase() !== 'none'; }; if (isProvided(config?.data?.oauth_con_id) || isProvided(config?.data?.oauthService)) { // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/core/src/Components/APICall/APICall.class.ts(1 hunks)
📝 Description
Updated the OAuth configuration check to ensure 'oauth_con_id' is not undefined before proceeding. This prevents potential issues when 'oauth_con_id' is missing from the config data and makes the first check true if it is undefined
🔗 Related Issues
🔧 Type of Change
✅ Checklist
Summary by CodeRabbit
Bug Fixes
Improvements