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

Expose more error information from http admin API #80

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

orbitalturtle
Copy link
Contributor

This PR aims to expose more information about the errors returned from the sensei api, to make things clearer for api users

Thought it would be good to get some feedback on the approach so far. But I would like to do the same for some of the other silenced errors in http/admin.rs, as well as get started on the http/node.rs errors

A little more specifically, this PR:

@johncantrell97
Copy link
Contributor

This is amazing and sorely needed. Thank you for working on this. I'll have to dig in a bit deeper but at first glance my only comment is can we try to keep axum/tonic and related content outside of "senseicore".

The idea of senseicore is to make it completely protocol agnostic so that it can be used as a library without any http/grpc dependencies.

I guess this might be a problem here though? Will you run into the 'you can't implement a trait for a type from another crate' issue?

Hm. If so then maybe it means we need to have some kind of "HttpError" struct that you can turn a regular SenseiError into and then implement IntoResponse for that?

Thoughts?

@orbitalturtle
Copy link
Contributor Author

@johncantrell97 Ahhh I see. That makes sense. I'll try moving things around!

@orbitalturtle orbitalturtle force-pushed the improve-err-handling branch 2 times, most recently from f337e97 to f1960bc Compare June 7, 2022 19:35
@orbitalturtle
Copy link
Contributor Author

@johncantrell97 moved the IntoResponse out of senseicore & added a HttpError struct. Will fix the clippy errors at some point too, but in the meantime probably ready for another look :)

@orbitalturtle orbitalturtle force-pushed the improve-err-handling branch 2 times, most recently from 745b63a to a9e0356 Compare June 10, 2022 04:03
@orbitalturtle orbitalturtle force-pushed the improve-err-handling branch from a9e0356 to 066d074 Compare June 10, 2022 04:06
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.

2 participants