Server crashed when webhook receives 400 status code response

[wxfred]

Description
If my nestjs server returns a 400 status code response to the onChange webhook, the hocuspocus will crash.

I see there is no catch clause in hocuspocus-webhook.esm.js

Is it suitable to add a catch like below?

async sendRequest(event, payload) {
  const json = JSON.stringify({ event, payload });
  return axios.post(this.configuration.url, json, { headers: { 'X-Hocuspocus-Signature-256': this.createSignature(json), 'Content-Type': 'application/json' } }).catch(err => console.log(err));
}

And, sendRequest uses axios to send requests, it seems all 2xx status code will be treated as a success, while others will throw a exception which crashes the entire server. The line 109 shows

if (response.status !== 200 || !response.data)
  return;

Maybe, don't hard code response.status !== 200 will be nice, because POST responses of nestjs will contain a 201 status code by default and axios considers it as a success.

Summary

1. add catch to axios.post
2. remove response.status !== 200

That's my own adverise, welcome to discuss.

Environment?

• Hocuspocus version: v2.11.2

[janthurau]

@wxfred I've just quickly checked but as far as I could see, all sendRequest calls are wrapped in try/catch with console.error as the error handler. Can you share the stack trace of the crash?

https://github.com/ueberdosis/hocuspocus/blob/main/packages/extension-webhook/src/index.ts#L122

[wxfred]

@janthurau Thanks for your prompt, I located the error, the original code didn't add async/await to axios, sendRequest returns a promise, and here are my changes

async onChange(data) {
    if (!this.configuration.events.includes(Events.onChange)) {
        return;
    }
    const save = async () => { // add async
        try {
            await this.sendRequest(Events.onChange, {  // add await
                document: this.configuration.transformer.fromYdoc(data.document),
                documentName: data.documentName,
                context: data.context,
                requestHeaders: data.requestHeaders,
                requestParameters: Object.fromEntries(data.requestParameters.entries()),
            });
        }
        catch (e) {
            console.error(`Caught error in extension-webhook: ${e}`);
        }
    };
    if (!this.configuration.debounce) {
        return save();
    }
    this.debounce(data.documentName, save);
}

BTW, I found that other hooks actually have async/await, the miss is only in onChange.

[janthurau]

uff. Thanks for reporting this! Will push a fix right away.