Skip to content

Return Response instead of Result<Response, hyper::Error> from juniper_hyper::graphql (and others) #881

Closed
@LukasKalbertodt

Description

@LukasKalbertodt

I was wondering why the public functions in juniper_hyper return a Result<Response, hyper::Error> instead of a simple Response. And indeed, looking into the code, it looks like they always return Ok. So clearly, right now, the Result is not needed.

Is Result used to plan for potential future scenarios where juniper_hyper would want to return errors? I'm not sure if that makes sense:

  • There is currently no public way to create a hyper::Error, so you could only forward errors returned by other hyper functions.
  • I don't see why juniper_hyper would ever use a hyper function returning an error. There are not too many of those (e.g. awaiting the Server results in a Result<(), hyper::Error>). Most errors that hyper::Error represents happen either when accepting/parsing the request or when sending the response: the juniper_hyper code is in between of that. Right now I don't see what hyper::Error those functions could reasonably return.
  • Other errors (GraphQL parse errors and such) are already mapped to a BAD REQUEST response, not an Err.

If hyper changes in the future, all this might change of course. But since updating the hyper version requires a major version bump in juniper_hyper anyway, I would think that right now, the public functions of juniper_hyper should return a Response directly.

Anything I'm missing?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions