Introduce Absolute_http specialization

[torinnd]

An RFC9110-compliance specialization of a [Uri.t]

Addresses #162

[avsm]

Thanks! This looks good to me, some notes in no order:

• needs some tests as well
• we're increasingly shifting to exception raising functions as the default, with the move towards direct-style interfaces in OCaml 5. In this case, the exception raised should probably be more specific (not Failure), and I wonder if the Result type should be the slow one (that is, implement the function to raise an effect, and wrap that for the Result version of the function rather than the other way around).

[talex5]

we're increasingly shifting to exception raising functions as the default, with the move towards direct-style interfaces in OCaml 5.

If you mean Eio's error handling policy then I'd like to clarify that this is for IO errors, where there is a large and unknown set of possible errors (e.g. depending on the platform). For those, you can't handle all individual errors and must usually propagate them by default, so exceptions typically make more sense.

Parsing is an interesting case. Eio.Buf_read.parse returns a result type, because it's common to want to handle parsing errors. But it wraps individual parsers that raise Failure on error, because plumbing result types through all the parsing code is slow.

This isn't my library, but this PR's choice of returning a result for the new of_uri function, and matching what the rest of the library does for of_string, seems fine to me. It would be good to document what of_string does on error, though.

[dinosaure]

This isn't my library, but this PR's choice of returning a result for the new of_uri function, and matching what the rest of the library does for of_string, seems fine to me. It would be good to document what of_string does on error, though.

I agree with that and ocaml-uri is relatively standalone. We should not use a wider error politic other than the one described into our website.

[avsm]

@talex5 wrote:

But it wraps individual parsers that raise Failure on error, because plumbing result types through all the parsing code is slow

Right, that's what made me think that the exception should be the fast one, as callers can then aggregate them into a single result.

But this can all be fixed later with affecting the interfaces in this PR. I'm good with merging this if you could add some tests, @torinnd

[torinnd]

I've taken a stab at putting a test harness together, but am bumping into the following ocaml-ci error: 2023-01-24 15:01.16: Job failed: Error from solver: Unix.Unix_error(Unix.EMFILE, "pipe", "")

I'm not sure if this is an issue with the PR or the CI tooling

[talex5]

2023-01-24 15:01.16: Job failed: Error from solver: Unix.Unix_error(Unix.EMFILE, "pipe", "") I'm not sure if this is an issue with the PR or the CI tooling

It's a problem with the CI certainly. The CI team are aware of it.

[talex5]

The new error is likely a problem with the PR however:

File "lib_test/test_runner.ml", line 727, characters 33-37:
727 |   eval_rfc9110_uris http_uris ~f:eval
                                       ^^^^
Error: This expression has type
         input:string -> (Uri.Absolute_http.t, string) result -> unit
       but an expression was expected of type
         input:string ->
         (Uri.Absolute_http.t, [ `Msg of string ]) result -> unit
       Type string is not compatible with type [ `Msg of string ] 

[tmcgilchrist]

@torinnd the CI issue is resolved, could you try rebasing this PR. Thanks

[torinnd]

Checks cleared! Thanks for getting the opam issue sorted @tmcgilchrist !

[torinnd]

@avsm are you open to merging this? I think we've sorted all of the feedback raised in this PR.

[avsm]

Merged, and released to opam. Thanks for the contribution!