refactoring and more tests

Author: kensnyder

packages/bunshine/benchmarks/content-length-bug.ts

@@ -0,0 +1,20 @@
+const server = Bun.serve({
+  fetch() {
+    return new Response(null, {
+      status: 200,
+      statusText: 'OK',
+      headers: {
+        'Content-Type': 'image/jpeg',
+        'Content-Length': '1000',
+      },
+    });
+  },
+});
+
+const resp = await fetch(server.url, { method: 'HEAD' });
+
+// should be 1000, but is actually 0
+const contentLength = resp.headers.get('Content-Length');
+console.log(`Content-length=${contentLength}`);
+
+server.stop(true);

packages/bunshine/src/Context/Context.spec.ts

@@ -19,89 +19,7 @@ describe('Context', () => {
     expect(c.now).toBeNumber();
     expect(c.url).toBeInstanceOf(URL);
     expect(c.url.pathname).toBe('/home');
-  });
-  it('should handle files', async () => {
-    const request = new Request('http://localhost/home.html');
-    const app = new HttpRouter();
-    const c = new Context(request, server, app);
-    const resp = await c.file(
-      `${import.meta.dir}/../../testFixtures/home.html`
-    );
-    expect(resp).toBeInstanceOf(Response);
-    expect(resp.headers.get('Accept-Ranges')).toBe('bytes');
-    const file = await resp.blob();
-    const text = await file.text();
-    expect(text).toBe('<h1>Welcome home</h1>\n');
-  });
-  it('should handle files with disposition="attachment', async () => {
-    const request = new Request('http://localhost/home.html');
-    const app = new HttpRouter();
-    const c = new Context(request, server, app);
-    const resp = await c.file(
-      `${import.meta.dir}/../../testFixtures/home.html`,
-      { disposition: 'attachment' }
-    );
-    expect(resp).toBeInstanceOf(Response);
-    expect(resp.headers.get('Content-Disposition')).toBe(
-      'attachment; filename="home.html"'
-    );
-    const file = await resp.blob();
-    const text = await file.text();
-    expect(text).toBe('<h1>Welcome home</h1>\n');
-  });
-  it('should handle files with range "bytes=0-3"', async () => {
-    const request = new Request('http://localhost/home.html', {
-      headers: { Range: 'bytes=0-3' },
-    });
-    const app = new HttpRouter();
-    const c = new Context(request, server, app);
-    const resp = await c.file(
-      `${import.meta.dir}/../../testFixtures/home.html`
-    );
-    expect(resp).toBeInstanceOf(Response);
-    const file = await resp.blob();
-    const text = await file.text();
-    expect(resp.status).toBe(206);
-    expect(text).toBe('<h1>');
-  });
-  it('should handle files with range "bytes=0-"', async () => {
-    const request = new Request('http://localhost/home.html', {
-      headers: { Range: 'bytes=0-' },
-    });
-    const app = new HttpRouter();
-    const c = new Context(request, server, app);
-    const resp = await c.file(
-      `${import.meta.dir}/../../testFixtures/home.html`
-    );
-    expect(resp).toBeInstanceOf(Response);
-    const file = await resp.blob();
-    const text = await file.text();
-    expect(text).toBe('<h1>Welcome home</h1>\n');
-  });
-  it('should handle files with range "bytes=0-999"', async () => {
-    const request = new Request('http://localhost/home.html', {
-      headers: { Range: 'bytes=0-999' },
-    });
-    const app = new HttpRouter();
-    const c = new Context(request, server, app);
-    const resp = await c.file(
-      `${import.meta.dir}/../../testFixtures/home.html`
-    );
-    expect(resp).toBeInstanceOf(Response);
-    expect(resp.status).toBe(200);
-  });
-  it('should handle files with range "bytes=-3"', async () => {
-    const request = new Request('http://localhost/home.html', {
-      headers: { Range: 'bytes=-3' },
-    });
-    const app = new HttpRouter();
-    const c = new Context(request, server, app);
-    const resp = await c.file(
-      `${import.meta.dir}/../../testFixtures/home.html`
-    );
-    expect(resp).toBeInstanceOf(Response);
-    expect(resp.status).toBe(206);
-    expect(await resp.text()).toBe('<h1>');
+    expect(c.locals).toBeTypeOf('object');
   });
   describe('server', () => {
     let c: Context;
@@ -157,15 +75,5 @@ describe('Context', () => {
       expect(resp.headers.get('Content-type')).toStartWith('application/json');
       expect(resp.headers.get('X-Hello')).toBe('World');
     });
-    // it('should include redirect(url)', () => {
-    //   const resp = redirect('/home');
-    //   expect(resp.headers.get('Location')).toBe('/home');
-    //   expect(resp.status).toBe(302);
-    // });
-    // it('should include redirect(url, status)', () => {
-    //   const resp = redirect('/home', 301);
-    //   expect(resp.headers.get('Location')).toBe('/home');
-    //   expect(resp.status).toBe(301);
-    // });
   });
 });

packages/bunshine/src/Context/Context.ts

@@ -74,8 +74,8 @@ export default class Context<
     return json.call(this, data, init);
   };
   /** A shorthand for `new Response(null, { headers: { Location: url }, status: 301 })` */
-  redirect = (url: string, status = 302) => {
-    return redirect(url, status);
+  redirect = (url: string, statusOrInit?: number | ResponseInit) => {
+    return redirect(url, statusOrInit);
   };
   /** A shorthand for `new Response(bunFile, fileHeaders)` plus range features */
   file = async (
@@ -89,6 +89,6 @@ export default class Context<
   };
   /** A shorthand for `new Response({ headers: { 'Content-type': 'text/event-stream' } })` */
   sse = (setup: SseSetupFunction, init: ResponseInit = {}) => {
-    return sse(this.request.signal, setup, init);
+    return sse.call(this, this.request.signal, setup, init);
   };
 }

packages/bunshine/src/HttpRouter/HttpRouter.spec.ts

@@ -1,20 +1,12 @@
 import type { Server } from 'bun';
-import { afterEach, beforeEach, describe, expect, it, spyOn } from 'bun:test';
-import EventSource from 'eventsource';
+import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
 import HttpRouter from './HttpRouter';
 
-type SseTestEvent = {
-  type: string;
-  data: string;
-  lastEventId?: string;
-  origin?: string;
-};
-
 // @ts-expect-error
 const server: Server = {};
 
 describe('HttpRouter', () => {
-  let port = 7500;
+  let port = 50100;
   describe('handlers', () => {
     let app: HttpRouter;
     let oldEnv: string | undefined;
@@ -480,135 +472,4 @@ describe('HttpRouter', () => {
       expect(await resp.text()).toBe('bar');
     });
   });
-  describe('sse', () => {
-    let app: HttpRouter;
-    let server: Server;
-    beforeEach(() => {
-      app = new HttpRouter();
-    });
-    afterEach(() => {
-      server.stop(true);
-    });
-    function sseTest({
-      port,
-      payloads,
-      event,
-      headers = {},
-    }: {
-      port: number;
-      payloads:
-        | Array<[string, any, string]>
-        | Array<[string, any]>
-        | Array<[string]>;
-      event: string;
-      headers?: Record<string, string>;
-    }): Promise<SseTestEvent[]> {
-      return new Promise((outerResolve, outerReject) => {
-        const events: SseTestEvent[] = [];
-        const readyToSend = new Promise((resolve, reject) => {
-          app.get('/sse', c => {
-            return c.sse(
-              send => {
-                resolve(() => {
-                  for (const payload of payloads) {
-                    // @ts-expect-error
-                    send(...payload);
-                  }
-                });
-              },
-              { headers }
-            );
-          });
-          app.onError(c => reject(c.error));
-          server = app.listen({ port });
-        }) as Promise<() => void>;
-        const readyToListen = new Promise((resolve, reject) => {
-          const stream = new EventSource(`http://localhost:${port}/sse`);
-          stream.addEventListener('error', evt => {
-            reject(evt);
-            stream.close();
-          });
-          stream.addEventListener(event, evt => {
-            events.push(evt);
-            if (events.length === payloads.length) {
-              outerResolve(events);
-              stream.close();
-            }
-          });
-          resolve(port);
-        }) as Promise<number>;
-        Promise.all([readyToSend, readyToListen])
-          .then(([doSend]) => doSend())
-          .catch(outerReject);
-      });
-    }
-    it('should handle unnamed data', async () => {
-      const events = await sseTest({
-        event: 'message',
-        port: port++,
-        payloads: [['Hello'], ['World']],
-      });
-      expect(events.length).toBe(2);
-      expect(events[0].data).toBe('Hello');
-      expect(events[1].data).toBe('World');
-    });
-    it('should send last event id and origin', async () => {
-      const events = await sseTest({
-        event: 'myEvent',
-        port: port++,
-        payloads: [
-          ['myEvent', 'hi1', 'id1'],
-          ['myEvent', 'hi2', 'id2'],
-        ],
-      });
-      expect(events.length).toBe(2);
-      expect(events[0].data).toBe('hi1');
-      expect(events[1].data).toBe('hi2');
-      expect(events[0].lastEventId).toBe('id1');
-      expect(events[1].lastEventId).toBe('id2');
-      expect(events[0].origin).toBe(`http://localhost:${port - 1}`);
-      expect(events[1].origin).toBe(`http://localhost:${port - 1}`);
-    });
-    it('should JSON encode data if needed', async () => {
-      const events = await sseTest({
-        event: 'myEvent',
-        port: port++,
-        payloads: [['myEvent', { name: 'Bob' }]],
-      });
-      expect(events.length).toBe(1);
-      expect(events[0].data).toBe('{"name":"Bob"}');
-    });
-    it('should warn when overriding some headers', async () => {
-      spyOn(console, 'warn').mockImplementation(() => {});
-      await sseTest({
-        event: 'myEvent',
-        port: port++,
-        payloads: [['myEvent', { name: 'Bob' }]],
-        headers: {
-          'Content-Type': 'text/plain',
-          'Cache-Control': 'foo',
-          Connection: 'whatever',
-        },
-      });
-      expect(console.warn).toHaveBeenCalledTimes(3);
-      // @ts-expect-error
-      console.warn.mockRestore();
-    });
-    it('should not warn if those headers are correct', async () => {
-      spyOn(console, 'warn').mockImplementation(() => {});
-      await sseTest({
-        event: 'myEvent',
-        port: port++,
-        payloads: [['myEvent', { name: 'Bob' }]],
-        headers: {
-          'Content-Type': 'text/event-stream',
-          'Cache-Control': 'no-cache',
-          Connection: 'keep-alive',
-        },
-      });
-      expect(console.warn).toHaveBeenCalledTimes(0);
-      // @ts-expect-error
-      console.warn.mockRestore();
-    });
-  });
 });

packages/bunshine/src/HttpRouter/HttpRouter.ts

@@ -226,17 +226,12 @@ export default class HttpRouter {
       context.params = match[1];
 
       try {
-        let result = handler(context, next);
+        let result = await handler(context, next);
         if (result instanceof Response) {
           return result;
+        } else {
+          return next();
         }
-        if (typeof result?.then === 'function') {
-          result = await result;
-          if (result instanceof Response) {
-            return result;
-          }
-        }
-        return next();
       } catch (e) {
         return errorHandler(e as Error);
       }

packages/bunshine/src/middleware/compression/compression.ts

@@ -35,7 +35,10 @@ export function compression(
   const resolvedOptions = { ...compressionDefaults, ...options };
   return async (context, next) => {
     const resp = await next();
-    if (!isCompressibleMime(resp.headers.get('Content-Type'))) {
+    if (
+      context.request.method === 'HEAD' ||
+      !isCompressibleMime(resp.headers.get('Content-Type'))
+    ) {
       return resp;
     }
     const accept = context.request.headers.get('Accept-Encoding') ?? '';

packages/bunshine/src/middleware/serveFiles/serveFiles.spec.ts

@@ -22,9 +22,7 @@ describe('serveFiles middleware', () => {
       return c.text('Error', { status: 500 });
     });
   });
-  afterEach(() => {
-    server.stop(true);
-  });
+  afterEach(() => server.stop(true));
   describe('files', () => {
     it('should serve file', async () => {
       app.get('/files/*', serveFiles(fixturesPath));
@@ -53,7 +51,7 @@ describe('serveFiles middleware', () => {
       const text = await resp.text();
       expect(text).toBe('');
       expect(resp.headers.get('content-length')).toBe('0');
-      expect(resp.status).toBe(204);
+      expect(resp.status).toBe(200);
     });
     it('should 404 if file does not exist', async () => {
       app.get('/files/*', serveFiles(fixturesPath));