Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to provide exception handler for timer's callback #20745

Merged
merged 4 commits into from
Apr 23, 2024

Conversation

vladsud
Copy link
Contributor

@vladsud vladsud commented Apr 19, 2024

Not much to add here...

@vladsud vladsud requested a review from a team April 19, 2024 01:37
@github-actions github-actions bot added area: loader Loader related issues public api change Changes to a public API base: main PRs targeted against main branch labels Apr 19, 2024
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Apr 19, 2024

@fluid-example/bundle-size-tests: +468 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 453 KB 453.09 KB +93 Bytes
azureClient.js 548.61 KB 548.71 KB +94 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 256.08 KB 256.17 KB +93 Bytes
fluidFramework.js 339.61 KB 339.61 KB No change
loader.js 132.13 KB 132.23 KB +94 Bytes
map.js 41.46 KB 41.46 KB No change
matrix.js 143.45 KB 143.45 KB No change
odspClient.js 516.51 KB 516.6 KB +94 Bytes
odspDriver.js 97.06 KB 97.06 KB No change
odspPrefetchSnapshot.js 41.93 KB 41.93 KB No change
sharedString.js 161.22 KB 161.22 KB No change
sharedTree.js 339.61 KB 339.61 KB No change
Total Size 3.15 MB 3.15 MB +468 Bytes

Baseline commit: efb82f8

Generated by 🚫 dangerJS against 5a0a532

@@ -138,7 +138,7 @@ export function setLongTimeout(timeoutFn: () => void, timeoutMs: number, setTime

// @internal
export class Timer implements ITimer {
constructor(defaultTimeout: number, defaultHandler: () => void, getCurrentTick?: () => number);
constructor(defaultTimeout: number, defaultHandler: () => void, exceptionHandler?: ((error: unknown) => void) | undefined);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we specified the type of error here as any, then we could keep the other error types preserved which you had to change below to unknown.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. unknown is safer than any, as any could be implicitly casted by compiler to any type, so it's easy to introduce a bug without noticing it.

The only casts I see is in UT and they are unrelated to this handler - they are there because compiler does not understand that clock.tick() has side-effects, so it thinks that exceptionCounter is always one (after first assert).

@vladsud vladsud merged commit c51f298 into microsoft:main Apr 23, 2024
30 checks passed
@vladsud vladsud deleted the TimerExceptions branch April 23, 2024 21:35
vladsud added a commit that referenced this pull request Apr 25, 2024
… using "read" connection. (#20817)

First step to enable Audience to have same behavior for "read" and "write" clients
- "connected" event raised when "self" is present in Audience.

Next step:
- client is caught up with latest ops.

For deeper description of work in this space, please see first PR below.

This is continuation of this work:
- #20703
- #20752
- #20704
- #20745
- #20766

And active PRs (do not block this PR, but make whole story more consistent for serialized / rehydrated containers):
- #20768
- #20767

Changeset added in previous PR in the same version (part of it should have been included in this PR, but made it earlier by accident):
[.changeset/cruel-trees-dance.md](https://github.com/microsoft/FluidFramework/pull/20215/files#diff-8f706b3e769f698985bea8fbf9bb3fb0a0a8798de53ad3f51cbace1dd3d1d2b2)
vladsud added a commit that referenced this pull request Apr 26, 2024
…d" event when using "read" connections. (#20854)

Enable Audience to have same behavior for "read" and "write" clients:
- "connected" event raised when we caught up on ops.

For deeper description of work in this space, please see first PR below.

This is continuation of this work:
- #20703
- #20752
- #20704
- #20745
- #20766
- #20817

And active PRs (do not block this PR, but make whole story more consistent for serialized / rehydrated containers):
- #20767

Changeset added in previous PR in the same version (part of it should have been included in this PR, but made it earlier by accident):
[.changeset/cruel-trees-dance.md](https://github.com/microsoft/FluidFramework/pull/20215/files#diff-8f706b3e769f698985bea8fbf9bb3fb0a0a8798de53ad3f51cbace1dd3d1d2b2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: loader Loader related issues base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants