Closed plisovyi closed 2 months ago
This is a huge PR! that is really hard to track the changes in. if you wanted to improve errors it could have been mush smaller PR.
Well, you are right. It is a big one, and just improving errors could be extracted into a much smaller PR, which I did here —https://github.com/wixplosives/engine/pull/2568; so please, let's review that one instead.
Next time, let's not even get into reviewing "huge PRs" if you think there's too much in the way of reviewing. If it is possible, it is not too hard to decouple only required changes into their own PR. And while indeed this is the very crucial core part, I think we should encourage changes and evolution by making sure there are no hiccups in the process; changes in 2568 could have been extracted 2 weeks ago and there'd be no need in this whole back and forth.
This PR aims to refactor/clean up Communications regarding debug logging and error handling in preparation for better processing of such errors during error monitoring. Errors would have proper names and messages attached to them with redacted information.
CommunicationOptions
as they are inferred all way back to class property definitionRemoteAPIServicesMapping
type and movedConfigEnvironmentRecord
to packages/core/src/com/types.tscommunication.ts
as messages make more sense near the action they describe and are not reused in multiple places; usedthis.log?.()
pattern to make sure log code doesn't run if debug is offcallMethod
method made private