refactor(lib): consolidate reqwest errors
#125
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
CodSpeed Performance ReportMerging #125 will not alter performanceComparing Summary
|
| Err(e) => Err(Error::RequestEncode(e)), | ||
| Err(_) => Err(Error::InvalidRequest(resp.text().await?)), |
There was a problem hiding this comment.
I think we should add a test for this.
E.g., sending a request that is too long. I don't know that is the limit of local servers, but the public API has a limit of 1500 characters, I think.
There was a problem hiding this comment.
Sure, I'll have a look
There was a problem hiding this comment.
Is that last commit there what you had in mind?
There was a problem hiding this comment.
Yes, it looks good @Rolv-Apneseth! Sorry, I didn't see the notification...
Could you fix the conflicts so I can run tests?
There was a problem hiding this comment.
I'm away at the moment but I can have a look when I'm back, hopefully on the weekend
There was a problem hiding this comment.
No issue, ping me when you're back ;-)
There was a problem hiding this comment.
@jeertmans Did a rebase, hopefully it's all good now
There was a problem hiding this comment.
Perfect, thanks! We will fix CodSpell's warnings later (we need to ignore them)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #125 +/- ##
==========================================
+ Coverage 19.06% 19.47% +0.40%
==========================================
Files 14 14
Lines 577 570 -7
==========================================
+ Hits 110 111 +1
+ Misses 467 459 -8 ☔ View full report in Codecov by Sentry. |
…ode` into just `Error::Reqwest`
…o long when checking either text or annotated data
Rolv-Apneseth
force-pushed
the
consolidate-reqwest-errors
branch
from
a81a453 to
5514c75
Compare
| let req = Request::default().with_text("je suis une poupee"); | ||
| assert!(client.check(&req).await.inspect_err(dbg_err).is_ok()); | ||
|
|
||
| // Too long | ||
| let req = Request::default().with_text("Repeat ".repeat(1500)); |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
Unpaired symbol: ‘"’ seems to be missing (EN_UNPAIRED_QUOTES)
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-parentheses
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_QUOTES?lang=en-US
Category: PUNCTUATION
| // Too long | ||
| let req = Request::default() | ||
| .with_data_str(&format!( | ||
| "{{\"annotation\":[{{\"text\": \"{}\"}}]}}", |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
Unpaired symbol: ‘"’ seems to be missing (EN_UNPAIRED_QUOTES)
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-parentheses
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_QUOTES?lang=en-US
Category: PUNCTUATION
jeertmans
changed the title
reqwest errorsreqwest errors
This removes
Error::RequestEncodeandError::RequestDecode. Also did some minor refactoring to remove excessive indentation, and implemented the test forError::Reqwest(though I think maybe that's not necessary - feels like testing implementation details).Note: I believe I accidentally made this branch from the main repo and not my fork - my bad. If that's an issue I can make a new PR.