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

[Supplier] chore: improve supplier not found error message #183

Merged
merged 4 commits into from
Nov 15, 2023

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Nov 14, 2023

Summary

Human Summary

Add context to the "key not found" error message that is returned when querying for a supplier which hasn't staked yet:
image

AI Summary

Summary generated by Reviewpad on 14 Nov 23 10:24 UTC

This pull request improves the error message seen by RPC consumers when the given supplier is not found on-chain. It adds a formatted message that includes the address of the supplier.

Issue

image

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make go_develop_and_test
  • Verify Localnet manually: See the instructions [here](TODO: add link to instructions)

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@bryanchriswhite bryanchriswhite added supplier Changes related to the Supplier actor relayminer Changes related to the Relayminer labels Nov 14, 2023
@bryanchriswhite bryanchriswhite self-assigned this Nov 14, 2023
@bryanchriswhite bryanchriswhite marked this pull request as ready for review November 14, 2023 10:25
@@ -52,7 +53,8 @@ func (k Keeper) Supplier(goCtx context.Context, req *types.QueryGetSupplierReque
req.Address,
)
if !found {
return nil, status.Error(codes.NotFound, "not found")
msg := fmt.Sprintf("supplier with address %q", req.GetAddress())
Copy link
Member

Choose a reason for hiding this comment

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

Going to think about how to iterate on this moving forward but just approving for now.

@bryanchriswhite bryanchriswhite merged commit 8f61392 into main Nov 15, 2023
8 checks passed
@bryanchriswhite bryanchriswhite deleted the chore/supplier-not-found-error branch November 15, 2023 08:36
bryanchriswhite added a commit that referenced this pull request Nov 16, 2023
* relayer/cli:
  More tiny comment updates
  Added a couple more comments
  Update some comments and TODOs
  Update the names and references to queryNode/sequencerNode/fullNode etc
  Update pkg/relayer/cmd/cmd.go
  [Test] Updating `relay.feature` to run curl command to enable E2E Relay Test (#178)
  Updated comments for post 177+179 work for okdas
  Update OpenAPI spec
  Update .gitignore
  chore: update comment
  chore: move shared dependency setup logic to shared pkg
  chore: cleanup flags and dependencies for appgateserver cmd
  [Supplier] chore: improve supplier not found error message (#183)
  [CI] Integrate E2E tests with GitHub CI (#152)
bryanchriswhite added a commit that referenced this pull request Nov 16, 2023
…-update

* merge/e2e_test/relay_x_relayer_cli:
  [LocalNet] Run Relayer and AppGateServer (#179)
  [Relay] E2E Relay Gaps (#177)
  More tiny comment updates
  Added a couple more comments
  Update some comments and TODOs
  Update the names and references to queryNode/sequencerNode/fullNode etc
  Update pkg/relayer/cmd/cmd.go
  [Test] Updating `relay.feature` to run curl command to enable E2E Relay Test (#178)
  Updated comments for post 177+179 work for okdas
  Update OpenAPI spec
  Update .gitignore
  chore: update comment
  chore: move shared dependency setup logic to shared pkg
  chore: cleanup flags and dependencies for appgateserver cmd
  [Supplier] chore: improve supplier not found error message (#183)
  [CI] Integrate E2E tests with GitHub CI (#152)
okdas pushed a commit that referenced this pull request Nov 14, 2024
* chore: improve error message seen by RPC consumers when the given supplier isn't on-chain

* fix: query supplier test

* chore: add todo comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relayminer Changes related to the Relayminer supplier Changes related to the Supplier actor
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants