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: context cancelled/deadline exceeded errors coming back internal #1803

Merged
merged 7 commits into from
Jun 29, 2023

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Jun 29, 2023

Fixes: #1801

  • checks for context.Canceled and context.DeadlineExceeded errors and returns proper GRPC code instead of Internal
  • moves ErrorUnaryInterceptor up the chain so its before any of our other custom middleware
  • Swaps out deprecated WithUnaryServerChain from grpc_middleware package to grpc.ChainUnaryInterceptor. The former just called the latter anyways

TODO

  • I want to check that our Interceptor ordering logic is correct and that the ErrorInterceptor is always the outer most

@markphelps markphelps requested a review from a team as a code owner June 29, 2023 01:03
@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Merging #1803 (d3f7d80) into main (3bf3255) will decrease coverage by 0.13%.
The diff coverage is 38.88%.

@@            Coverage Diff             @@
##             main    #1803      +/-   ##
==========================================
- Coverage   70.41%   70.28%   -0.13%     
==========================================
  Files          58       58              
  Lines        5550     5563      +13     
==========================================
+ Hits         3908     3910       +2     
- Misses       1419     1427       +8     
- Partials      223      226       +3     
Impacted Files Coverage Δ
internal/cmd/grpc.go 0.00% <0.00%> (ø)
internal/server/auth/middleware.go 90.80% <11.11%> (-9.20%) ⬇️
internal/server/middleware/grpc/middleware.go 69.96% <100.00%> (+1.81%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

Looks good. We could always setup whipt against it with a low timeout.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 29, 2023
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jun 29, 2023
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 29, 2023
@markphelps markphelps changed the title fix(wip): context cancelled/deadline exceeded errors coming back internal fix: context cancelled/deadline exceeded errors coming back internal Jun 29, 2023
@markphelps markphelps merged commit c12967b into main Jun 29, 2023
@markphelps markphelps deleted the mp/context-cancelled-errs branch June 29, 2023 17:20
markphelps added a commit that referenced this pull request Jul 3, 2023
* main: (66 commits)
  chore: Labs link (#1813)
  chore: add feedback link to app footer (#1812)
  chore(deps-dev): bump eslint-plugin-playwright in /ui (#1818)
  chore(deps): bump react-router-dom from 6.14.0 to 6.14.1 in /ui (#1819)
  chore(deps-dev): bump @typescript-eslint/parser in /ui (#1817)
  chore(deps-dev): bump ts-jest from 29.1.0 to 29.1.1 in /ui (#1816)
  chore(deps): bump google.golang.org/protobuf from 1.30.0 to 1.31.0 (#1815)
  chore(deps): bump google.golang.org/protobuf in /_tools (#1814)
  fix: Fix validate command (#1811)
  chore: fix deprecated archive replacement (#1807)
  docs: add charlesoconor as a contributor for doc (#1810)
  Update import in the README (#1809)
  fix: context cancelled/deadline exceeded errors coming back internal (#1803)
  chore: add  to codecov ignore (#1804)
  chore(deps): bump react-router-dom from 6.13.0 to 6.14.0 in /ui (#1794)
  chore: add docker to devcontainer so can run tests (#1799)
  chore(deps-dev): bump @typescript-eslint/parser in /ui (#1797)
  chore(deps): bump @tanstack/react-table from 8.9.2 to 8.9.3 in /ui (#1795)
  chore(deps): bump swr from 2.1.5 to 2.2.0 in /ui (#1793)
  chore: address fixes from PR feedback
  ...
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.

[FLI-466] Do not return codes Internal and Unauthenticated for context timeout errors
2 participants