Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(connector): fix Aliyun SMS connector error handling #1227

Merged
merged 6 commits into from
Jul 7, 2022

Conversation

darcyYe
Copy link
Contributor

@darcyYe darcyYe commented Jun 24, 2022

Summary

Fix error handling of requesting SmsSend API.

Testing

UTs.

@linear
Copy link

linear bot commented Jun 24, 2022

LOG-3201 Fix Aliyun SMS error handling

阿里云短信请求的返回值有些奇怪,有些错误会正常返回 response,解析 response body 之后通过 查看 code 能知道错误信息;同时有的时候也会直接在请求时抛错,这两种情况都要考虑。

之前没有发现会在请求时直接抛错的情况。

@github-actions
Copy link

github-actions bot commented Jun 24, 2022

COMPARE TO master

Total Size Diff 📈 +776 Bytes

Diff by File
Name Diff
packages/connector-aliyun-sms/src/index.ts 📈 +776 Bytes

@darcyYe darcyYe marked this pull request as draft June 24, 2022 06:57
@gao-sun
Copy link
Member

gao-sun commented Jul 3, 2022

What's the status of this?

@darcyYe darcyYe force-pushed the yemq-log-3201-aliyun-sms-error-handling branch from c0b4685 to 5a0282c Compare July 4, 2022 13:23
@darcyYe darcyYe marked this pull request as ready for review July 4, 2022 13:40
@darcyYe darcyYe force-pushed the yemq-log-3201-aliyun-sms-error-handling branch from 0182dee to b5c594d Compare July 4, 2022 13:43
@darcyYe darcyYe changed the title fix(connector-aliyun-sms): fix error handling fix(connector): fix Aliyun SMS connector error handling Jul 4, 2022
Copy link
Contributor

@wangsijie wangsijie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems we don't need to call errorHandler twice.

Copy link
Contributor

@wangsijie wangsijie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, we can do a refactor to fix complexity issue later.

@darcyYe darcyYe merged commit d9ba729 into master Jul 7, 2022
@darcyYe darcyYe deleted the yemq-log-3201-aliyun-sms-error-handling branch July 7, 2022 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants