Add more encryption properties to PostHog #12415
Conversation
|
Sorry, forgot to create this PR as a draft |
uhoreg
added
the
T-Task
This comment was marked as resolved.
This comment was marked as resolved.
|
@BillCarsonFr what does the |
|
merged latest develop |
|
Oh, it still needs an Element Web reviewer. I've requested review from Robin and Rich because those were the people GitHub had randomly chosen initially. |
|
I think that I've addressed all the requested changes. |
| * | ||
| * @param {function} fn The tracking function, which will be called when failures | ||
| * are tracked. The function should have a signature `(count, trackedErrorCode) => {...}`, | ||
| * where `count` is the number of failures and `errorCode` matches the output of `errorCodeMapFn`. | ||
| * are tracked. The function should have a signature `(trackedErrorCode, rawError, properties) => {...}`, |
There was a problem hiding this comment.
[I'm not convinced it's correct that this function is called when failures are "tracked", but that's an existing problem I guess.]
src/DecryptionFailureTracker.ts
Outdated
| * @param matrixEvent the event that was decrypted | ||
| * | ||
| * @param nowTs the current timestamp | ||
| */ | ||
| public eventDecrypted(e: MatrixEvent, nowTs: number): void { |
There was a problem hiding this comment.
this should probably not be public any more?
There was a problem hiding this comment.
On the one hand, I agree. On the other hand, I'm not particularly excited about littering DecryptionFailureTracker-test.ts with a ton of @ts-ignores
src/DecryptionFailureTracker.ts
Outdated
| /** Callback for when an event is decrypted. | ||
| * | ||
| * This function should be called after a decryption attempt on an event. It | ||
| * should be called whether the decryption is successful or not. |
There was a problem hiding this comment.
As an alternative to making this private:
| /** Callback for when an event is decrypted. | |
| * | |
| * This function should be called after a decryption attempt on an event. It | |
| * should be called whether the decryption is successful or not. | |
| /** Callback for when an event is decrypted, successfully or otherwise. | |
| * | |
| * It is unnecessary to call this function explicitly: it is automatically called on a decryption event. | |
| * It is only exposed for testing. |
(I'd probably vote in favour of the ts-ignore dance for unit tests. You could probably even encapsulate it in a single helper method in the test. But at this point I think we should just land this rather than attempting to gold-plate it.)
There was a problem hiding this comment.
I actually like the idea of encapsulating it in a helper function, so I'm doing that. (I'm also updating the doc comment to say that it's called by the event handler, rather than saying that it should be called.)
Fixes element-hq/element-web#27168
Checklist
public/exportedsymbols have accurate TSDoc documentation.