-
Notifications
You must be signed in to change notification settings - Fork 135
Description
I've got a situation where my GitHub webhook endpoint needs to handle both chat messages, and GitHub webhook events via the same webhook URL.
I've managed to implement this like so:
export function createExtendedGitHubAdapter(config: GitHubAdapterConfig) {
return new ExtendedGitHubAdapter(config, app);
}
class ExtendedGitHubAdapter extends GitHubAdapter {
constructor(config: GitHubAdapterConfig) {
super(config);
}
override async handleWebhook(
request: Request,
options?: WebhookOptions,
): Promise<Response> {
const clone = request.clone();
const response = await super.handleWebhook(request, options);
if (!response.ok) {
return response;
}
// Handle webhook events with octokit
await octokit.webhooks.receive({
id,
name,
payload: await clone.json(),
} as EmitterWebhookEvent);
...So first pipe through the chat sdk, if all is ok, then pipe through the octokit webhook handlers. This works.
However, if an event from GitHub comes through (e.g. installation.created), it errors:
[ERROR] TypeError: Cannot read properties of undefined (reading 'owner')
at ExtendedGitHubAdapter.handleWebhook (/node_modules/@chat-adapter/github/src/index.ts:381:14)
There is no repository.owner property on this (and probably other event types).
The fix could be as simple as if (installationId && this.isMultiTenant && payload.repository) {, however I'm not 100% sure if this covers all bases?
For now, the (not ideal) workaround is:
try {
response = await super.handleWebhook(request, options);
} catch {
// Noop...
}Additionally, it's be good if the octokit property as protected rather than private so we don't need to create our own octokit instance with the config.