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(redis): Fixed a get/set error not returning #31

Closed
wants to merge 1 commit into from

Conversation

Luckyboys
Copy link
Contributor

Since the previous "store.Store" interface did not return an error, the upper caller could not know that the storage error occurred. This problem will cause the upper caller to know the specific error message when a storage error occurs during the generation or verification of the verification code.

@codecov
Copy link

codecov bot commented Jul 3, 2019

Codecov Report

Merging #31 into master will increase coverage by 0.45%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
+ Coverage   88.23%   88.68%   +0.45%     
==========================================
  Files          11       11              
  Lines        1071     1096      +25     
==========================================
+ Hits          945      972      +27     
+ Misses         79       78       -1     
+ Partials       47       46       -1
Impacted Files Coverage Δ
store/memory.go 100% <100%> (+4.44%) ⬆️
store/redis.go 100% <100%> (ø) ⬆️
captcha.go 96.34% <100%> (+1.6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87c9c59...04b27aa. Read the comment docs.

@Luckyboys Luckyboys force-pushed the feature_log_redis_error branch 4 times, most recently from 927a16d to 3d0de76 Compare July 3, 2019 12:33
Since the previous "store.Store" interface did not return an error, the upper caller could not know that the storage error occurred. This problem will cause the upper caller to know the specific error message when a storage error occurs during the generation or verification of the verification code.
@Luckyboys
Copy link
Contributor Author

Luckyboys commented Jul 3, 2019

@mojocn 我们在实际使用过程中,发现如果Redis发生写入或者获取出错时,上层没办法进行修复性操作,比如重试或者重新调用GenerateCaptcha函数来重新生成一张新的图使得校验值存储到Redis中。

因此我们希望把错误可以往上层传达,使得上层可以根据情况记录日志、重试、或者其他希望处理的方法。

如果方法的命名上面有什么更好的规范的话,请告知我们,我们及时进行修改。
希望此功能可以对更多使用该库的用户带来便利和帮助。
谢谢~

@JJJJJJJerk
Copy link
Collaborator

这个库我想尽量使用官方标准库,不想过多的应用第三发redis, 再说golang redis库有好几种, 也知道用户喜欢应用哪一种, 项目中如果出现两种redis库,这个项目给人不简洁的感觉.
redis-store使用者自己实现就好了.如果出现错误返回空字符串就好了, 这个库不负责自定义redis错误,项目在自定义redis-stroe set/get方法的自己应该处理打印(处理)redis错误.

@JJJJJJJerk JJJJJJJerk closed this Jul 4, 2019
@JJJJJJJerk JJJJJJJerk reopened this Jul 4, 2019
@JJJJJJJerk
Copy link
Collaborator

这个项目代码比较简单,自己复制一份自己改也可以. 非常感谢你的建议.
关于命名的问题我会在后续修改, 但是方法名还是要兼容之前的

@JJJJJJJerk JJJJJJJerk closed this Jul 4, 2019
@Luckyboys Luckyboys deleted the feature_log_redis_error branch July 4, 2019 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants