-
Notifications
You must be signed in to change notification settings - Fork 11
Web standard APIs (cross-platform support) #11
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
Conversation
80232d5
to
bf51714
Compare
Tests are now set up and automated via GitHub actions (all tests are passing). We use |
This PR is now fully web API compatible, including the browser. We have browser tests for chromium, firefox, and webkit (via vitest + playwright) that are also automated in CI and passing. Browser tests use a const [clientDuplex, serverDuplex] = createDuplexPair<Uint8Array>();
const db = new PGlite();
new PostgresConnection(serverDuplex, {
async onStartup() {
await db.waitReady;
},
async onMessage(data, { isAuthenticated }) {
if (!isAuthenticated) {
return;
}
return await db.execProtocolRaw(data);
},
});
// read/write `clientDuplex` In the tests we actually use an in-browser version of import pg from '@nodeweb/pg';
const { Client } = pg;
const [clientDuplex, serverDuplex] = createDuplexPair<Uint8Array>();
const db = new PGlite();
new PostgresConnection(serverDuplex, {
async onStartup() {
await db.waitReady;
},
async onMessage(data, { isAuthenticated }) {
if (!isAuthenticated) {
return;
}
return await db.execProtocolRaw(data);
},
});
const client = new Client({
user: 'postgres',
stream: socketFromDuplexStream(clientDuplex),
});
await client.connect();
const res = await client.query("select 'Hello world!' as message");
const [{ message }] = res.rows;
expect(message).toBe('Hello world!');
await client.end();
|
TLS logic is confirmed working via
|
PGlite extended query tests are now passing via fixes in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stellar work @gregnr 👏
Just a couple of questions and little suggestions, but nothing blocking for merging! 🫡
This is a somewhat major refactor that changes
pg-gateway
to use only web standard APIs.Why?
Cross-platform support. Building on web streams and web crypto means that
pg-gateway
can work on other platforms beyond Node.js like Deno, custom runtimes, and even the browser.What changed?
There are 5 main changes in the PR:
Uint8Array
Web streams
PostgresConnection
now accepts aReadableStream
andWritableStream
instead of a Node.jsSocket
. Specifically it accepts aDuplexStream
, which is simply an object containing theReadableStream
andWritableStream
:The design for this duplex interface was not arbitrary - it is a pattern encouraged in the web streams spec, and is used by most duplex-like implementations, like Deno's
Conn
(TCP socket),TransformStream
, etc.A helper function exists for each platform to simplify converting between their native socket and a
DuplexStream
.eg. Node.js
Socket
:or Deno's
Conn
:Note that these helper functions are imported from separate entry points (package
exports
) so that their native dependencies don't interfere with bundlers if you don't use them.Worth considering: The above helper functions were actually really simple to make and might not be needed. For example, to convert from a Node.js
Socket
toDuplexStream
is actually as simple as calling:and Deno's
Conn
actually already implementsDuplexStream<Uint8Array>
, so no conversion was needed (just wrapped).We do a bit of extra work though in
fromNodeSocket()
to support TLS upgrades (more on this below), and other future implementations might not be so simple, so it may be worth keeping these helper functions for consistency.TLS upgrades
Abstracting
Socket
into a standardDuplexStream
is great, but it makes TLS upgrades tricky since this logic is platform specific. Right now this is solved by movingupgradeTls()
to the consumer who is now responsible for TLS upgrades by implementing this method. The method should return a newDuplexStream
that operates on top of the new encrypted channel (like a Node.jsTLSSocket
).To make this easy for Node.js users,
fromNodeSocket()
implements this method for you using our previous TLS upgrade logic. Unfortunately Deno does not yet offer APIs to upgrade a TCP connection to TLS from the server side, so TLS is not yet supported there (but likely in the future).Async iterables
Async iterables are a modern way to work with streams. For example Deno uses them to handle connections on TCP servers and listening for new data within each stream.
It turns out that
ReadableStream
implementsAsyncIterable
, so we can now easily use this within our own implementation. Instead of listening for asocket.on('data')
event, we now iterate over the readable:A nice side effect from this is that reading data is now pull-based instead of push-based. This means we no longer need to worry about pausing and resuming the socket like we did before since new data won't arrive until we ask for it (will get buffered under the hood).
In addition to the above, this PR modifies all downstream logic to use async generators. For example,
handleClientMessage()
is an async generator that calls yield whenever it wants to write data back to the client (instead ofthis.sendData()
):Nested methods can also yield data and will be forwarded by parent functions:
Web crypto
Another area that needed to change was auth, since it heavily relied on Node's crypto APIs. Most functions we used had a direct 1-to-1 mapping to Web Crypto, with the exception of MD5 which is not implemented (probably for security reasons). To support MD5, we use Deno's
@std/crypto
library which implements missing algorithms like MD5 using Web Assembly.Uint8Array
Up until now we've used Node's
Buffer
to represent binary data, but it's not a web standard. This PR modifies every occurrence ofBuffer
to useUint8Array
instead. Any missing methods (likeconcat()
,toString('base64')
) are implemented using Deno's@std/bytes
and@std/encoding
standard libraries.Tests
We finally have some tests 🎉 Given all the refactors, testing was the only way to be sure things don't break. We were also long overdue for creating tests. This PR introduces baseline tests for Node and Deno, with the goal to increase test coverage in subsequent PRs.