codeant-ai-for-open-source[bot] commented on PR #36924: URL: https://github.com/apache/superset/pull/36924#issuecomment-3714943568
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36924/files#diff-354576cc4b18946c489144939e5503a27833b0b5016e3f311c87f624d4001aacR142-R157'><strong>Return type change</strong></a><br>getPermalink now resolves to an object { key, url } instead of a string URL. Callers that previously expected a string will break. Verify all call sites (including tests and downstream code) are updated to handle PermalinkResult or adapt via a compatibility layer.<br> - [ ] <a href='https://github.com/apache/superset/pull/36924/files#diff-98c17f45146f0c20c04fbd289cda3e4416b0750bbc36a550c1ae39b3a10b3d08R254-R264'><strong>Missing port start</strong></a><br>The code registers an RPC method with `ourPort.defineMethod('resolvePermalinkUrl', ...)` but does not call `ourPort.start()` beforehand. Other places (e.g. `observeDataMask`) call `ourPort.start()` before `defineMethod`. Without starting the port, incoming calls from the iframe may not be delivered to the handler.<br> - [ ] <a href='https://github.com/apache/superset/pull/36924/files#diff-bc0d7dbe0cec52aa7e76dc729138550344b9160acf3f62a98104bce717a15ba6R71-R92'><strong>Unhandled host callback errors</strong></a><br>The code calls the host-provided callback via Switchboard.get without error handling. If the host callback throws or Switchboard.get rejects, permalink generation could fail and surface exceptions to the embed consumer. Add a fallback and/or catch block to ensure the default Superset URL is used on errors.<br> - [ ] <a href='https://github.com/apache/superset/pull/36924/files#diff-354576cc4b18946c489144939e5503a27833b0b5016e3f311c87f624d4001aacR160-R210'><strong>Tests / Mocks impact</strong></a><br>Tests or mocks that stub SupersetClient.post may return different shapes (e.g., returning the mock input). Ensure unit tests and any code that mocks SupersetClient.post are updated to return an object with a .json.{key,url} shape. Update or add tests that assert the new PermalinkResult.<br> - [ ] <a href='https://github.com/apache/superset/pull/36924/files#diff-98c17f45146f0c20c04fbd289cda3e4416b0750bbc36a550c1ae39b3a10b3d08R258-R263'><strong>No error handling / validation</strong></a><br>The host-provided `resolvePermalinkUrl` is invoked directly and its result returned to the iframe. If the host callback throws or returns a non-string (or an invalid URL), this could result in unhandled promise rejections or surprising values sent to the iframe. The SDK should guard the call, catch errors, and validate/fallback to Superset's default behaviour.<br> </td></tr> </table> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
