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

Refactor handleAcceptH #1685

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

Conversation

ErnestKz
Copy link

Hi, the root of this PR is to change:

    handleAcceptH :: Proxy list -> AcceptHeader -> a -> Maybe (ByteString, ByteString)

to

    handleAcceptH :: Proxy list -> AcceptHeader -> Maybe (ByteString, a -> ByteString)

Here, a is the value returned by the handler, so this is supposed to run after a successful route match. However, the Maybe indicates whether the AcceptHeader is one that we are accepting specified by the Proxy list.

This is a problem because we want to know if the AcceptHeader contains a valid value before we run the handler on it and extract the a. In the original version of the function, we need to run the handler first, and only then we find out if the request contains an appropriate header.

The consequences of this is that we sort force the user to do an additional separate check and then some form of an unsafe coercions/ awkard error throw in order to bypass the Maybe.

Example of awkward error throw:

Nothing -> FailFatal err406 -- this should not happen (checked before), so we make it fatal if it does

If it seems like a good idea to introduce this API change. I'm happy to update the rest of the projects in this repo.

Cheers.

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.

1 participant