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

Implement ALPN support for the Deno adapter. #199

Merged
merged 9 commits into from
Jan 20, 2022
Merged

Implement ALPN support for the Deno adapter. #199

merged 9 commits into from
Jan 20, 2022

Conversation

1st1
Copy link
Member

@1st1 1st1 commented Nov 16, 2021

Depends on edgedb/deno@a95fef9

@1st1 1st1 requested a review from jaclarke November 16, 2021 03:35
@1st1
Copy link
Member Author

1st1 commented Nov 16, 2021

My test file (t.ts):

import {createClient} from "./edgedb-deno/mod.ts";

async function main() {
  let conn = createClient({
    dsn: "_localdev", tlsSecurity: "insecure",
    //tlsCAFile: '/home/yury/.local/share/edgedb/_localdev/edbtlscert.pem'
  });
  console.log('=1');
  await conn.ensureConnected();
  console.log('=2');

  console.log(await conn.querySingle('select 32323'));
  console.log('=3');
}

main()

I was running it with

yarn compileForDeno && \
   ../../deno/target/debug/deno run --allow-all --unstable \
       --unsafely-ignore-certificate-errors t.ts

Assuming that Deno was cloned into the ../../deno directory from the https://github.com/edgedb/deno/tree/alpn branch.

To build Deno locally I followed https://deno.land/manual@v1.16.1/contributing/building_from_source. It's basically just cloning the repo with all submodules and running cargo build.

@1st1
Copy link
Member Author

1st1 commented Nov 16, 2021

@jaclarke James, please take a look at this. Specifically check if we need any more APIs for a complete edgedb-deno client. The adapter.deno.ts / tls.connect() function needs some work to pass all connection options. Then try testing without --unsafely-ignore-certificate-errors.

Lastly, by default Deno tries to connect to ::1 at least on my machine, but edgedb-server only listens on 127.0.0.1 (and doesn't listen on IPv6). Passing 127.0.0.1 to deno explicitly doesn't work for some reason (a limitation of rustls? I didn't spend much time debugging that). Assuming that I'll merge edgedb/edgedb#3187 tonight, run edb server -I "::1" to allow Deno to be able to connect using TLS.

@jaclarke
Copy link
Member

Specifically check if we need any more APIs for a complete edgedb-deno client.

The client will mostly work without these, but to fully match the behaviour of edgedb-js it looks like we will need a few more:

  • To support tlsSecurity values insecure and no_host_verification we need equivalents of the rejectUnauthorized and checkServerIdentity options
  • ref/unref for allowing deno to exit while idle connections are open

@jaclarke
Copy link
Member

by default Deno tries to connect to ::1 at least on my machine

For me I'm assuming localhost still resolves to 127.0.0.1, since it works without edb server -I "::1", so I guess this depends on the user's system.

@1st1
Copy link
Member Author

1st1 commented Nov 16, 2021

we need equivalents of the rejectUnauthorized

Looks like that won't happen: denoland/deno#1371

@1st1
Copy link
Member Author

1st1 commented Nov 16, 2021

I've filed a PR to add ALPN support: denoland/deno#12786

@jaclarke jaclarke marked this pull request as ready for review December 22, 2021 14:10
@1st1 1st1 merged commit 4031cb1 into master Jan 20, 2022
@1st1 1st1 deleted the deno_alpn branch January 20, 2022 20:24
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.

None yet

2 participants