dolittle / JavaScript.SDK

Dolittle JavaScript SDK
https://dolittle.io
MIT License
5 stars 2 forks source link

Fix eventhandler async methods #59

Closed jakhog closed 3 years ago

jakhog commented 3 years ago

Summary

Fixes a bug where async Event Handler methods were not properly awaited (and thus also exceptions ignored) if they did any async work. In the process, also found a bug related to delete results returned from async Projection and Embedding methods, that would not be handled correctly.

Fixed

jakhog commented 3 years ago

Changing the Error types to any is because we are catching things thrown potentially from user-code, so we have no way of knowing that it actually is an Error or just whatever other type. The code was not building (ts -b) because of that in one place, so I also went through and changed it consistently so that we don't assume any wrong types.

I will double check that the createOnMethod handles it, but it did return a T | Promise<T> before my changes, so I just assumed that it would work. Will comment back on my findings.

jakhog commented 3 years ago

The on method for Embeddings and Projections is called here: https://github.com/dolittle/JavaScript.SDK/blob/a6d6c012b34efbd76f51793d285b4c54a3ebb52d/Source/embeddings/Internal/EmbeddingProcessor.ts#L211 https://github.com/dolittle/JavaScript.SDK/blob/a6d6c012b34efbd76f51793d285b4c54a3ebb52d/Source/projections/Internal/ProjectionProcessor.ts#L191

The result will be the returned value from the code I changed in createOnMethod, so it is safe to return a promise here.