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

Revert "feat(Container): Add IContainer.closedWithError to access the error that closed or disposed the container" #23399

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

jikim-msft
Copy link
Contributor

@jikim-msft jikim-msft commented Dec 23, 2024

Description

 
Reverts #23364

This change appears to cause errors in our end to end stress tests.

@Copilot Copilot bot review requested due to automatic review settings December 23, 2024 18:20
@jikim-msft jikim-msft requested review from a team as code owners December 23, 2024 18:20
@github-actions github-actions bot added area: loader Loader related issues area: tests Tests to add, test infrastructure improvements, etc changeset-present public api change Changes to a public API base: main PRs targeted against main branch labels Dec 23, 2024

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

packages/test/test-end-to-end-tests/src/test/container.spec.ts:694

  • Verify that the change in the error message from "dispose error" to "expected" is correct and does not introduce any unintended side effects.
container.dispose(new DataCorruptionError("expected", {}));

packages/test/test-end-to-end-tests/src/test/container.spec.ts:747

  • Verify that the change in the error message from "close error" to "expected" is correct and does not introduce any unintended side effects.
container.close(new DataCorruptionError("expected", {}));
Copy link
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170142 links
    1596 destination URLs
    1825 URLs ignored
       0 warnings
       0 errors


@jikim-msft jikim-msft enabled auto-merge (squash) December 23, 2024 18:53
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

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

Code Coverage Summary

↓ packages.loader.container-loader.src:
Line Coverage Change: -0.01%    Branch Coverage Change: -0.02%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 91.33% 91.31% ↓ -0.02%
Line Coverage 94.71% 94.70% ↓ -0.01%

Baseline commit: 58619c3
Baseline build: 313974
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: -460 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 470.67 KB 470.71 KB +35 Bytes
azureClient.js 567.53 KB 567.35 KB -186 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 266.44 KB 266.46 KB +14 Bytes
fluidFramework.js 439.15 KB 439.17 KB +14 Bytes
loader.js 134.45 KB 134.23 KB -221 Bytes
map.js 42.68 KB 42.69 KB +7 Bytes
matrix.js 150.48 KB 150.49 KB +7 Bytes
odspClient.js 534.98 KB 534.8 KB -186 Bytes
odspDriver.js 99.48 KB 99.5 KB +21 Bytes
odspPrefetchSnapshot.js 43.04 KB 43.05 KB +14 Bytes
sharedString.js 166.62 KB 166.63 KB +7 Bytes
sharedTree.js 429.63 KB 429.63 KB +7 Bytes
Total Size 3.42 MB 3.42 MB -460 Bytes

Baseline commit: 58619c3

Generated by 🚫 dangerJS against af102a5

@jikim-msft jikim-msft merged commit 6780fdb into main Dec 23, 2024
41 checks passed
@jikim-msft jikim-msft deleted the revert-23364-container-close-prop branch December 23, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: loader Loader related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch changeset-present public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants