Vincit/tarn.js

`createFail` event handler not called when resource creation times out

sam-perez opened this issue · 3 comments

I've been debugging an issue in production and have been trying to log out tarn lifecycle events. After reading a confusing set of logs and diving deep into this library, it seems that we're only firing the createFail event when the resource creator rejects, but not when the PendingOperation rejects with a timeout. Why is this the case?

For reference, here's the code I'm talking about:

this._executeEventHandlers('createFail', eventId, err);

Suggested alternative:

      .catch(err => {
        this._executeEventHandlers('createFail', eventId, err);

        if (pendingCreate.isRejected) {
          return null;
        }

        remove(this.pendingCreates, pendingCreate);

        // Not returned on purpose.
        pendingCreate.reject(err);
        return null;
      });

It seems to me that since this creation has failed (due to timeout or otherwise), we should notify.

I will check this out and make sure it has a test.

Looks like that handler doesn't seem to be fired at all when create times out, but the correct place to add the event handler firing was easy... though looks like while writing the tests I did find another worse bug, which may cause resource to not be free'ed in case if destroy() fails with timeout... It will be kind of silent failure and the pool hangs.

I was before looking into this in a process to write document about pool logic and specifying exactly how cases with timeouts etc. should behave to be able to rewrite the code in a way that it doesn't look so confusing with so many very hard to spot unintuitive corner cases.

Though I hope to get the release problem fixed before the complete rehaul.

Ok, huh found it out that it was just my test causing that problem 😅 Pushing PR soonish.