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

refactor(core): Add ErrorKind InvalidInput to indicate users input error #2637

Merged
merged 2 commits into from
Jul 14, 2023

Conversation

dqhl76
Copy link
Member

@dqhl76 dqhl76 commented Jul 14, 2023

If user try to seek for a invalid position, we will give an Unexpected Error. Unexpected Error used to represent OpenDAL don't know what happened here. I think it may be better to use a new error kind InvalidInput to represent this situation.

    /// OpenDAL don't know what happened here, and no actions other than just
    /// returning it back. For example, s3 returns an internal service error.
    Unexpected,

before:

  2023-07-14T06:37:19.152933Z ERROR opendal::services: service=memory operation=Reader::seek path=bfae56eb-ae99-4ca6-b000-4bdad69c85e1 read=0 -> data read failed: Unexpected (permanent) at Reader::seek => invalid seek to a negative or overflowing position

Context:
    service: memory
    path: bfae56eb-ae99-4ca6-b000-4bdad69c85e1

    at core/src/layers/logging.rs:1108

after:

  2023-07-14T07:50:38.166551Z ERROR opendal::services: service=memory operation=Reader::seek path=0fca6ade-8d96-44fc-ac5b-86a370b26048 read=0 -> data read failed: InvalidInput (permanent) at Reader::seek => invalid seek to a negative or overflowing position

Context:
    service: memory
    path: 0fca6ade-8d96-44fc-ac5b-86a370b26048

    at core/src/layers/logging.rs:1108
    

related:#2636 (comment)
fix: #2636

Signed-off-by: dqhl76 <[email protected]>
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks!

@Xuanwo
Copy link
Member

Xuanwo commented Jul 14, 2023

Would you like to submit a new PR to add the test case in our behavior test?

@dqhl76
Copy link
Member Author

dqhl76 commented Jul 14, 2023

Would you like to submit a new PR to add the test case in our behavior test?

Sure!

@Xuanwo Xuanwo changed the title fix: make seek invalid error more clear refactor(core): Add ErrorKind InvalidInput to indicate users input error Jul 14, 2023
@Xuanwo Xuanwo merged commit 74db254 into apache:main Jul 14, 2023
@dqhl76 dqhl76 deleted the dqhl76-invalidseek branch July 14, 2023 08:09
ClSlaid added a commit to ClSlaid/opendal that referenced this pull request Jul 14, 2023
* feat(binding/lua): add rename and create_dir operator function by @oowl in apache#2564
* feat(services/azblob): support sink by @suyanhanx in apache#2574
* feat(services/gcs): support sink by @suyanhanx in apache#2576
* feat(services/oss): support sink by @suyanhanx in apache#2577
* feat(services/obs): support sink by @suyanhanx in apache#2578
* feat(services/cos): impl sink by @suyanhanx in apache#2587
* feat(service): Support stat for Dropbox by @Zheaoli in apache#2588
* feat(services/dropbox): impl create_dir and polish error handling by @suyanhanx in apache#2600
* feat(services/dropbox): Implement refresh token support by @Xuanwo in apache#2604
* feat(service/dropbox): impl batch delete by @suyanhanx in apache#2606
* feat(CI): set Kvrocks test for service redis by @suyanhanx in apache#2613
* feat(core): object versioning APIs by @suyanhanx in apache#2614
* feat(oay): actually read configuration from `oay.toml` by @messense in apache#2615
* feat(services/webdav): impl sink by @suyanhanx in apache#2622
* feat(services/fs): impl Sink for Fs by @Ji-Xinyou in apache#2626
* feat(core): impl `delete_with` on blocking operator by @suyanhanx in apache#2633
* feat(bindings/C): add support for list in C binding by @Ji-Xinyou in apache#2448
* refactor(core): Add ErrorKind InvalidInput to indicate users input error by @dqhl76 in apache#2637
* fix(doc): fix codeblock rendering by @xxchan in apache#2592
* fix(service/minitrace): should set local parent by @andylokandy in apache#2620
* fix(service/minitrace): update doc by @andylokandy in apache#2621
* doc(bindings/haskell): add module document by @silver-ymz in apache#2566
* docs: Update license related comments by @Prashanth-Chandra in apache#2573
* docs: add hdfs namenode High Availability related troubleshoot by @wcy-fdu in apache#2601
* docs: polish release doc by @PsiACE in apache#2608
* docs(blog): add Apache OpenDAL(Incubating): Access Data Freely by @PsiACE in apache#2607
* docs(RFC): Object Versioning by @suyanhanx in apache#2602
* ci: Disable bindings/java deploy for now by @tisonkun in apache#2560
* ci: Disable the failed stage-release job instead by @tisonkun in apache#2561
* ci: add haddock generator for haskell binding by @silver-ymz in apache#2569
* ci(binding/lua): add luarocks package manager support by @oowl in apache#2558
* build(deps): bump predicates from 2.1.5 to 3.0.1 by @dependabot in apache#2583
* build(deps): bump tower-http from 0.4.0 to 0.4.1 by @dependabot in apache#2582
* build(deps): bump chrono from 0.4.24 to 0.4.26 by @dependabot in apache#2581
* build(deps): bump redis from 0.22.3 to 0.23.0 by @dependabot in apache#2580
* build(deps): bump cbindgen from 0.24.3 to 0.24.5 by @dependabot in apache#2579
* ci: upgrade hawkeye to v3 by @tisonkun in apache#2585
* ci(services/webdav): Setup integration test for nextcloud by @Xuanwo in apache#2631
* chore: add haskell binding link to website by @silver-ymz in apache#2571
* chore: fix cargo warning for resolver by @xxchan in apache#2590
* chore: bump log to 0.4.19 by @xxchan in apache#2591
* chore(deps): update deps to latest version by @suyanhanx in apache#2596
* chore: Add release 0.38.0 to download by @PsiACE in apache#2597
* chore(service/minitrace): automatically generate span name by @andylokandy in apache#2618

* @Prashanth-Chandra made their first contribution in apache#2573
* @andylokandy made their first contribution in apache#2618

**Full Changelog**: apache/opendal@v0.38.0...v0.38.1

Signed-off-by: ClSlaid <[email protected]>
@ClSlaid ClSlaid mentioned this pull request Jul 14, 2023
Xuanwo pushed a commit that referenced this pull request Jul 14, 2023
* Bump to version 0.38.1

* feat(binding/lua): add rename and create_dir operator function by @oowl in #2564
* feat(services/azblob): support sink by @suyanhanx in #2574
* feat(services/gcs): support sink by @suyanhanx in #2576
* feat(services/oss): support sink by @suyanhanx in #2577
* feat(services/obs): support sink by @suyanhanx in #2578
* feat(services/cos): impl sink by @suyanhanx in #2587
* feat(service): Support stat for Dropbox by @Zheaoli in #2588
* feat(services/dropbox): impl create_dir and polish error handling by @suyanhanx in #2600
* feat(services/dropbox): Implement refresh token support by @Xuanwo in #2604
* feat(service/dropbox): impl batch delete by @suyanhanx in #2606
* feat(CI): set Kvrocks test for service redis by @suyanhanx in #2613
* feat(core): object versioning APIs by @suyanhanx in #2614
* feat(oay): actually read configuration from `oay.toml` by @messense in #2615
* feat(services/webdav): impl sink by @suyanhanx in #2622
* feat(services/fs): impl Sink for Fs by @Ji-Xinyou in #2626
* feat(core): impl `delete_with` on blocking operator by @suyanhanx in #2633
* feat(bindings/C): add support for list in C binding by @Ji-Xinyou in #2448
* refactor(core): Add ErrorKind InvalidInput to indicate users input error by @dqhl76 in #2637
* fix(doc): fix codeblock rendering by @xxchan in #2592
* fix(service/minitrace): should set local parent by @andylokandy in #2620
* fix(service/minitrace): update doc by @andylokandy in #2621
* doc(bindings/haskell): add module document by @silver-ymz in #2566
* docs: Update license related comments by @Prashanth-Chandra in #2573
* docs: add hdfs namenode High Availability related troubleshoot by @wcy-fdu in #2601
* docs: polish release doc by @PsiACE in #2608
* docs(blog): add Apache OpenDAL(Incubating): Access Data Freely by @PsiACE in #2607
* docs(RFC): Object Versioning by @suyanhanx in #2602
* ci: Disable bindings/java deploy for now by @tisonkun in #2560
* ci: Disable the failed stage-release job instead by @tisonkun in #2561
* ci: add haddock generator for haskell binding by @silver-ymz in #2569
* ci(binding/lua): add luarocks package manager support by @oowl in #2558
* build(deps): bump predicates from 2.1.5 to 3.0.1 by @dependabot in #2583
* build(deps): bump tower-http from 0.4.0 to 0.4.1 by @dependabot in #2582
* build(deps): bump chrono from 0.4.24 to 0.4.26 by @dependabot in #2581
* build(deps): bump redis from 0.22.3 to 0.23.0 by @dependabot in #2580
* build(deps): bump cbindgen from 0.24.3 to 0.24.5 by @dependabot in #2579
* ci: upgrade hawkeye to v3 by @tisonkun in #2585
* ci(services/webdav): Setup integration test for nextcloud by @Xuanwo in #2631
* chore: add haskell binding link to website by @silver-ymz in #2571
* chore: fix cargo warning for resolver by @xxchan in #2590
* chore: bump log to 0.4.19 by @xxchan in #2591
* chore(deps): update deps to latest version by @suyanhanx in #2596
* chore: Add release 0.38.0 to download by @PsiACE in #2597
* chore(service/minitrace): automatically generate span name by @andylokandy in #2618

* @Prashanth-Chandra made their first contribution in #2573
* @andylokandy made their first contribution in #2618

**Full Changelog**: v0.38.0...v0.38.1

Signed-off-by: ClSlaid <[email protected]>

* refactor: bump npm version

Signed-off-by: ClSlaid <[email protected]>

* chore: include #2634

Signed-off-by: 蔡略 <[email protected]>

---------

Signed-off-by: ClSlaid <[email protected]>
Signed-off-by: 蔡略 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Reader's seek operation incorrectly panic when the parameter is not valid
2 participants