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

[Feature Request]: blob_get_all could return empty Vec on "blob not found" #3192

Open
bkolad opened this issue Feb 20, 2024 · 5 comments · May be fixed by #3223
Open

[Feature Request]: blob_get_all could return empty Vec on "blob not found" #3192

bkolad opened this issue Feb 20, 2024 · 5 comments · May be fixed by #3223
Assignees
Labels
enhancement New feature or request external Issues created by non node team members

Comments

@bkolad
Copy link

bkolad commented Feb 20, 2024

Implementation ideas

The blob_get_all RPC method yields a Result<Vec<Blob>, Error>. When no blobs are published under a specified namespace and height, I am getting the forllowing error:

error is: ErrorObject { code: ServerError(1), message: "getting blobs for namespace(00000000000000000000000000000000000000736f762d746573742d70): blob: not found\nblob: not found", data: None }.

Missing blobs feels more like a standard scenario rather than an error. Currently there is no clean way to distinguish between a missing blob or some other HTTP error, as we can't pattern match on ErrorObject. I think it would be more appropriate to return an empty Vec in this case.

version: celestia-node:v0.12.0

@bkolad bkolad added the enhancement New feature or request label Feb 20, 2024
@github-actions github-actions bot added the external Issues created by non node team members label Feb 20, 2024
@jcstein
Copy link
Member

jcstein commented Feb 20, 2024

thanks for the issue @bkolad!

@renaynay
Copy link
Member

cc @tuxcanfly

had similar concerns

@jcstein
Copy link
Member

jcstein commented Feb 26, 2024

it is misleading

@ramin
Copy link
Collaborator

ramin commented Feb 28, 2024

cc @vgonkivs

@vgonkivs vgonkivs self-assigned this Feb 28, 2024
@Wondertan
Copy link
Member

This makes sense for the GetAll. The other methods should still return the error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request external Issues created by non node team members
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants