fix: safely extract error message from unknown error type - #2724
fix: safely extract error message from unknown error type#2724dajiaohuang wants to merge 2 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.
|
|
Thanks for the contribution! Before we can merge this, we need @dajiaohuang to sign the Salesforce Inc. Contributor License Agreement. |
| } catch (_) { | ||
| // failed to parse the response body as JSON | ||
| data = { ok: false, error: new TextDecoder().decode(buffer) }; | ||
| } |
There was a problem hiding this comment.
I see this is a duplicate of your other PR, https://github.com/slackapi/node-slack-sdk/pull/2723/changes. I'm going to close the other one
There was a problem hiding this comment.
Same nit as posted to the other PR, could move const text = new TextDecoder().decode(buffer); above to avoid decoding twice on error
There was a problem hiding this comment.
And the need for signing the CLA
|
Closing as a duplicate of https://github.com/slackapi/node-slack-sdk/pull/2725/changes#r3927012436 |
2 participants
Summary
The code in
packages/oauth/src/install-provider.tsassumed that caught errors (e) are always Error objects and accessede.messagedirectly. Ifewas a primitive value (string, number) or undefined, this could result in undefined being passed toGenerateInstallUrlError.Problem
At line 407:
If
eis not an Error object (e.g.,throw "something went wrong"orthrow null), then(e as any).messagewould beundefined.Fix
Used the same safe pattern already used elsewhere in this file (line 289):
This ensures a valid string is always passed to
GenerateInstallUrlError, whethereis an Error object or a primitive value.Testing
npm test --workspace=packages/oauth