webhooks.js icon indicating copy to clipboard operation
webhooks.js copied to clipboard

Middleware changes

Open tg44 opened this issue 2 years ago • 5 comments

fixes #682

There are three changes in this commit.

  • I added an option to match paths with regex
  • I pass down the original request to the handlers
  • ~~I typed the request and the response (as close as the code uses it)~~

The third mod could be opinionated. It matches the http interface and can be implemented in other HTTP frameworks too. Quick and dirty example for sunder;

import { Context } from "sunder";
import { ResponseData } from "sunder/util/response";
import { HeadersShorthands } from "sunder/context";

class WrappedRequest {
  request: Request & HeadersShorthands;
  headers: Record<string, string | string[] | undefined>;
  body: any;
  finished: Promise<unknown>;
  url: string;
  method: string;
  ctx: Context;
  setEncoding(enc: string) {};
  on(event: string, callback: (data: any) => void) {};

  constructor(ctx: Context) {
    const request = ctx.request;
    this.request = request;
    this.headers = Object.fromEntries(Array.from(request.headers.entries()));
    this.finished = request.json().then(async (d) => {
        this.body = d;
    });
    this.url = request.url;
    this.method = request.method;
    this.ctx = ctx;
  }
}

class WrappedResponse {
  response: ResponseData;

  set statusCode(value: number) {
    this.response.status = value;
  }

  end(body: string) {
    this.response.body = body;
  }

  writeHead(status: number, headers: Record<string, string>) {
    this.response.status = status;
    Object.entries(headers).map(([k, v]) => {
      this.response.headers.append(k, v);
    });
  }

  constructor(response: ResponseData) {
    this.response = response;
  }
}

export async function githubWebhooksMiddleware(ctx: Context, next: Function) {
  try {
    const request = new WrappedRequest(ctx);
    await request.finished;
    return webhooksMiddleware(
      request,
      new WrappedResponse(ctx.response),
      next
    );
  } catch (e) {
    console.log(e);
  }
}

tg44 avatar Jun 26 '22 22:06 tg44

can you update the README please?

gr2m avatar Jun 26 '22 23:06 gr2m

@gr2m now?

tg44 avatar Jun 27 '22 08:06 tg44

@timrogers @gr2m ping, I think I fixed/reverted all the concerns.

tg44 avatar Jul 06 '22 19:07 tg44

Hey @tg44 , im using fastify and was wondering if this pr could help me integrate with it??

amitgilad3 avatar Nov 29 '22 17:11 amitgilad3

@amitgilad3 this is the relevant WOT about what this PR does and what it doesn't. I think it would be nice to provide the described MiddlewareIOHelper and if we would implement that, probably fastify would be easy to add too. As you can see, we didn't merged this into, so I stopped working on this repo (bcs I already had the hack I needed for my app. I just wanted to improve the lib so others should not thinker with this when they want the same thing as I).

I think the current implementation of this lib uses the interfaces described here. If you can mock/wrap these functions to fastify specific, you can probably write a middleware yourself.

tg44 avatar Nov 30 '22 10:11 tg44