-
Notifications
You must be signed in to change notification settings - Fork 2
Ditto Web Chat - Fetch Attachment Error Fix #98
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
Conversation
🟢 Test Coverage Report -
|
| Metric | Coverage | Status |
|---|---|---|
| 🟢 Lines | 87.35% | green |
| 🟢 Statements | 87.35% | green |
| 🟢 Functions | 92.45% | green |
| 🟢 Branches | 86.1% | green |
📊 View Detailed Coverage Report
ℹ️ Coverage Thresholds
- 🟢 Excellent (≥ 80%)
- 🟡 Good (60-79%)
- 🟠 Fair (40-59%)
- 🔴 Poor (< 40%)
🟢 Test Coverage Report -
|
| Metric | Coverage | Status |
|---|---|---|
| 🟢 Lines | 91.89% | green |
| 🟢 Statements | 91.89% | green |
| 🟢 Functions | 85.98% | green |
| 🟢 Branches | 88.56% | green |
📊 View Detailed Coverage Report
ℹ️ Coverage Thresholds
- 🟢 Excellent (≥ 80%)
- 🟡 Good (60-79%)
- 🟠 Fair (40-59%)
- 🔴 Poor (< 40%)
|
| Severity | Count |
|---|---|
| 🔴 Critical | 0 |
| 🟠 High | 0 |
| 🟡 Medium | 1 |
| 🔵 Low | 0 |
| Total | 1 |
📋 Vulnerability Details
- CVE-2025-64718 (MEDIUM) in
js-yaml:3.14.2- Description: js-yaml is a JavaScript YAML parser and dumper. In js-yaml 4.1.0 and below, it's possible for an attacker to modify the prototype of the result of a parsed yaml document via prototype pollution ( proto ). All users who parse untrusted yaml documents may be impacted. The problem is patched in js-yaml 4.1.1. Users can protect against this kind of attack on the server by using node --disable-proto=delete or deno (in Deno, pollution protection is on by default).
- CVSS: 5.3
ℹ️ How to fix vulnerabilities
- Update vulnerable dependencies to patched versions
- Run
npm audit fixornpm audit fix --forcein the root directory - Check for alternative packages if updates aren't available
- Review and update your
package.jsonandpackage-lock.json
|
| Severity | Count |
|---|---|
| 🔴 Critical | 0 |
| 🟠 High | 0 |
| 🟡 Medium | 1 |
| 🔵 Low | 0 |
| Total | 1 |
📋 Vulnerability Details
- CVE-2025-64718 (MEDIUM) in
js-yaml:3.14.2- Description: js-yaml is a JavaScript YAML parser and dumper. In js-yaml 4.1.0 and below, it's possible for an attacker to modify the prototype of the result of a parsed yaml document via prototype pollution ( proto ). All users who parse untrusted yaml documents may be impacted. The problem is patched in js-yaml 4.1.1. Users can protect against this kind of attack on the server by using node --disable-proto=delete or deno (in Deno, pollution protection is on by default).
- CVSS: 5.3
ℹ️ How to fix vulnerabilities
- Update vulnerable dependencies to patched versions
- Run
npm audit fixornpm audit fix --forcein the root directory - Check for alternative packages if updates aren't available
- Review and update your
package.jsonandpackage-lock.json
…/web-chat-fetchattachment
| '@dittolive/ditto-chat-ui': patch | ||
| --- | ||
|
|
||
| Added retry and exponential back off mechanism in the fetchAttachment to resolve error during initial render. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if this is an obvious question, I am just coming back after some time off and lacking a little bit of context, but what exactly was the problem? The way this reads, it seems like we are "fixing" a React lifecycle issue by adding retries/backoff to the fetch operation? I'd like to better understand what the root issue was during initial render
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Aaron,
During the initial load, the application was encountering an error in fetchAttachment. This issue has already been resolved as part of the Cleanup PR, which is now merged into main.
I just double-checked this - would it be okay for me to go ahead and close this PR?
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the root issue was resolved, I think that satisfies my initial concern. I'm not opposed to having retries/backoff logic here, more so I am curious what the actual cause of the error was. Do you have any context you can share, maybe from the other PR where it was corrected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Aaron,
To investigate the Tailwind conflicts issue, I created a new project outside our workspace and added separate modules for comments and chat to verify the behavior. At the time of creating this setup, the singleton chat store initialization was not in place, and Yuvaraj was working on that part in parallel. I had also installed the npm packages via a local folder.
After the singleton initialization was introduced, I encountered the attachment fetch issue during the first initialization, which I believe was due to the local npm cache and locally linked instances. Adding a retry resolved the issue at that time.
Later, I force-refreshed the packages, cleared all caches, and performed a fresh install directly from the npm repository. With this setup, the issue no longer occurs, even with different modules and multiple initializations within the same project.
fetchAttachmentto resolve error during initial render.