fix: use consistent URL validation pattern in IncomingWebhook - #2725
fix: use consistent URL validation pattern in IncomingWebhook#2725dajiaohuang wants to merge 4 commits into
Conversation
…ponse
The JSON.parse at line 802 was not wrapped in a try-catch, which could
cause an unhandled exception if the response body is not valid JSON.
This is inconsistent with the similar operation at line 811 which is
properly wrapped.
Added try-catch to handle parse failures gracefully, returning
{ ok: false, error: <error message> } instead of throwing.
Before this fix, the code assumed `e` is an Error object and accessed `e.message` directly. If `e` was a primitive value or undefined, this could result in undefined being passed to GenerateInstallUrlError. Now we use the same pattern as line 289 in this file: `e instanceof Error ? e.message : String(e)` This ensures a valid string is always passed to GenerateInstallUrlError.
The URL validation in IncomingWebhook used `if (url === undefined)` which only catches undefined values. WebhookTrigger.ts uses the more robust `if (!url)` which catches undefined, null, and empty string. For consistency and better validation, updated IncomingWebhook to use the same pattern as WebhookTrigger.ts.
|
|
Thanks for the contribution! Before we can merge this, we need @dajiaohuang to sign the Salesforce Inc. Contributor License Agreement. |
|
👋 Hey @dajiaohuang! Before we review this PR I'm wondering if it'd be possible to sign the CLA above? 🏛️ ✨ |
| }, | ||
| ) { | ||
| if (url === undefined) { | ||
| if (!url) { |
There was a problem hiding this comment.
I see this PR is also a duplicate of your other PR, #2724. I will close the other one. In the future, do not spam our repo with multiple PRs for the same changes.
3 participants
Summary
The URL validation in
IncomingWebhookusedif (url === undefined)which only catches undefined values. Meanwhile,WebhookTriggerin the same package uses the more robustif (!url)which catches undefined, null, and empty string.Problem
At line 74 in
IncomingWebhook.ts:This check would not catch
nullor empty string""values.Fix
Changed to use the same pattern as
WebhookTrigger.ts:This ensures consistent validation across the webhook package and catches all falsy values.
Testing
npm test --workspace=packages/webhook