ravendb / ravendb-nodejs-client

RavenDB node.js client
MIT License
63 stars 32 forks source link

RDBC-829 allow to run on cloudflare using compatibility_flags instead of old node_compat #425

Closed ml054 closed 3 months ago

ngbrown commented 1 month ago

@ml054 Will there be an example of how to configure Cloudflare Worker's compatibility_flags? Also, previously there was a customFetch property defined with comments that it is for Cloudflare. Is that still needed?

ml054 commented 1 month ago

@ngbrown we will update docs after the release of 6.0

ngbrown commented 3 weeks ago

Hi @ml054, I'm trying to run the most recent 6.0 branch with Cloudflare Pages / Cloudflare Workers and am getting an import error on node:zlib.

With the following configuration in wrangler.toml:

compatibility_flags = ["nodejs_compat"]

I get the following error when running locally:

X [ERROR] service core:user:proj: Uncaught Error: No such module "node:zlib". imported from "138rxz8yt84.js" X [ERROR] MiniflareCoreError [ERR_RUNTIME_FAILURE]: The Workers runtime failed to start. There is likely additional logging output above.

The wrangler.toml setting of node_compat = true isn't compatible with Pages, thus the above configuration. There's no mention of zlib in the compatibility notes: https://developers.cloudflare.com/workers/configuration/compatibility-dates/

If you switch to the web standard of CompressionStream("gzip"), note that Cloudflare Pages/Workers have an interesting issue around using .pipeThrough() and the stream being closed too early, so it has to be manually managed.

Since compression is optional, by making the node:zlib import dynamic, I was able to issue a query while running in miniflare.

 src/Documents/BulkInsert/BulkInsertWriterBase.ts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/Documents/BulkInsert/BulkInsertWriterBase.ts b/src/Documents/BulkInsert/BulkInsertWriterBase.ts
index c59e721b..8a270ab5 100644
--- a/src/Documents/BulkInsert/BulkInsertWriterBase.ts
+++ b/src/Documents/BulkInsert/BulkInsertWriterBase.ts
@@ -1,7 +1,7 @@
 import { IDisposable } from "../../Types/Contracts.js";
 import { Buffer } from "node:buffer";
 import { pipeline, Readable } from "node:stream";
-import { createGzip, Gzip } from "node:zlib";
+import type { Gzip } from "node:zlib";
 import { promisify } from "node:util";
 import { TypeUtil } from "../../Utility/TypeUtil.js";

@@ -95,6 +95,7 @@ export class BulkInsertWriterBase implements IDisposable {

     public async ensureStream(compression: boolean) {
         if (compression) {
+            const { createGzip } = await import("node:zlib")
             this.compressedStream = createGzip();
             pipeline(this.requestBodyStream, this.compressedStream, TypeUtil.NOOP);
         }

Update: Before anyone gets their hopes up, no the RavenDB JS client doesn't currently work in Cloudflare Pages because binding to an MTLS certificate (mtls_certificates) isn't available yet in Pages. However, this issue does exist exactly the same in Cloudflare Workers, and I did get it working there with the above patch.

ml054 commented 3 weeks ago

@ngbrown Yes, you are right. I've introduced regression when unifying compression. I try apply dynamic imports to allow RavenDB in CloudFlare Pages.

For mtls it was working in the past (for workers). I didn't yet test mtls after latest changes to v6.0 branch. This is on our roadmap.