Skip to content

Conversation

@GiveMe-A-Name
Copy link
Collaborator

@GiveMe-A-Name GiveMe-A-Name commented Mar 21, 2025

Description:

Enhance errors reporting

SWC api try_with_handler only return anyhow!(msg) Err, it'a will lose many message about error stack, level ...,
To express more error message in swc, we can return Vec<Diagnostic> in try_with_handler;

try_with_handler_diagnostic return a Result<Ret, Vec<Diagnostic>>. Layer user can using diagnostic.code to distinguish what kind error is.

  • Added miette as a dependency for improved error handling and reporting.
  • Updated diagnostic handling to utilize miette for pretty printing errors.
  • Introduced a new ThreadSafetyDiagnostics to manage and store diagnostics.
  • Refactored emitters to support the new diagnostic structure and ensure compatibility with miette.
  • Enhanced error handling in various components to provide clearer and more informative error messages.

BREAKING CHANGE:

The api try_with_handler would not return anyhow!(msg).
It's will return TWithDiagnosticArray as it's Err. Then you can take diagnostics from TWithDiagnosticArray or get pretty string from it.

- pub fn try_with_handler<F, Ret>(cm: Lrc<SourceMap>, config: HandlerOpts, op:F) -> Result<Ret, anyhow::error>;
+ pub fn try_with_handler<F, Ret>(cm: Lrc<SourceMap>, config: HandlerOpts, op:F) -> Result<Ret, TWithDiagnosticArray>;
/// A wrapper around a value that also contains a list of diagnostics.
/// It helps swc error system migrate to the new error reporting system.
pub struct TWithDiagnosticArray {
    diagnostics: Vec<SwcDignostic>,
   ....
}

impl WrapperDiagnostics {
    pub fn diagnostics(&self) -> &[Diagnostic] {
        &self.diagnostics
    }

    pub fn to_pretty_error(&self) -> anyhow::Error {
        ....
    }


   pub fn to_pretty_string(&self) -> String {
      .... 
    }
}

Related issue (if exists):

#10192

@GiveMe-A-Name GiveMe-A-Name requested review from a team as code owners March 21, 2025 08:32
@GiveMe-A-Name GiveMe-A-Name marked this pull request as draft March 21, 2025 08:32
@changeset-bot
Copy link

changeset-bot bot commented Mar 21, 2025

🦋 Changeset detected

Latest commit: 0d98cd8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 21, 2025

CodSpeed Performance Report

Merging #10241 will degrade performances by 1.71%

Comparing GiveMe-A-Name:enhance/improve-error-recovery (0d98cd8) with main (fcf280f)

Summary

⚡ 2 improvements
❌ 3 regressions
✅ 147 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
serialization of serde 2.8 µs 2.8 µs -1.05%
es/lints/libs/d3 27.9 ms 27.6 ms +1.05%
es/lints/libs/terser 29.2 ms 29.7 ms -1.71%
es/minifier/libs/terser 401.5 ms 405.7 ms -1.05%
es/target/es2015 1.3 ms 1.2 ms +2.46%

@GiveMe-A-Name GiveMe-A-Name force-pushed the enhance/improve-error-recovery branch from f9f2397 to d27e639 Compare March 24, 2025 09:41
@GiveMe-A-Name GiveMe-A-Name marked this pull request as ready for review March 26, 2025 06:01
@GiveMe-A-Name GiveMe-A-Name marked this pull request as draft March 26, 2025 06:09
@kdy1
Copy link
Member

kdy1 commented Mar 26, 2025

Actually, a breaking change is fine until next version of swc_core is a breaking change anyway

@GiveMe-A-Name
Copy link
Collaborator Author

Actually, a breaking change is fine until next version of swc_core is a breaking change anyway

Got,I will update code later.

@GiveMe-A-Name GiveMe-A-Name force-pushed the enhance/improve-error-recovery branch from a2bebf7 to c962f90 Compare March 28, 2025 02:22
@GiveMe-A-Name GiveMe-A-Name marked this pull request as ready for review March 31, 2025 03:27
@kdy1 kdy1 added this to the Planned milestone Mar 31, 2025
@GiveMe-A-Name GiveMe-A-Name requested a review from kdy1 April 1, 2025 07:39
@GiveMe-A-Name GiveMe-A-Name force-pushed the enhance/improve-error-recovery branch from 21b984d to 7ec66ba Compare April 1, 2025 08:28
@kdy1 kdy1 self-assigned this Apr 1, 2025
@kdy1 kdy1 changed the base branch from main to dev April 1, 2025 09:20
@GiveMe-A-Name GiveMe-A-Name force-pushed the enhance/improve-error-recovery branch from 7ec66ba to 15292bb Compare April 1, 2025 10:45
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Btw, I love this PR :)

@GiveMe-A-Name GiveMe-A-Name force-pushed the enhance/improve-error-recovery branch from 0400dc0 to ba67496 Compare April 8, 2025 08:35
@GiveMe-A-Name GiveMe-A-Name requested a review from a team as a code owner April 8, 2025 08:35
@CLAassistant
Copy link

CLAassistant commented Apr 8, 2025

CLA assistant check
All committers have signed the CLA.

- Added `miette` as a dependency for improved error handling and reporting.
- Updated diagnostic handling to utilize `miette` for pretty printing errors.
- Introduced a new `DiagnosticCollection` to manage and store diagnostics.
- Refactored emitters to support the new diagnostic structure and ensure compatibility with `miette`.
- Enhanced error handling in various components to provide clearer and more informative error messages.
- Removed the `to_pretty_string` function and related error handling logic from various files to streamline error reporting.
- Replaced `DiagnosticCollection` with `DiagnosticWriter` for better management of diagnostics.
- Updated error handling in `swc_error_reporters` to utilize the new structure.
- Deleted the unused `json_emitter` module to clean up the codebase.
- Enhanced the `BufferedEmitter` for improved diagnostic output in testing scenarios.
@GiveMe-A-Name GiveMe-A-Name force-pushed the enhance/improve-error-recovery branch from 6cd8b72 to 923a434 Compare April 8, 2025 09:11
…`In practice this would nearly always result in OpenSSL treating the properties as an empty string`
@GiveMe-A-Name GiveMe-A-Name requested a review from kdy1 April 8, 2025 11:04
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Thank you so much!

@kdy1 kdy1 merged commit 96a7415 into swc-project:dev Apr 9, 2025
165 checks passed
kdy1 added a commit that referenced this pull request Apr 9, 2025
…10241)

# Description

SWC api `try_with_handler` only return anyhow!(msg) Err, it'a will lose many message about error stack, level ...,
To express more error message in swc, we can return `Vec<Diagnostic>` in `try_with_handler`;

try_with_handler_diagnostic return a `Result<Ret, Vec<Diagnostic>>`. Later user can using `diagnostic.code` to distinguish what kind error is.




- Added `miette` as a dependency for improved error handling and reporting.
- Updated diagnostic handling to utilize `miette` for pretty printing errors.
- Introduced a new `ThreadSafetyDiagnostics` to manage and store diagnostics.
- Refactored emitters to support the new diagnostic structure and ensure compatibility with `miette`.
- Enhanced error handling in various components to provide clearer and more informative error messages.



# BREAKING CHANGE:

The api `try_with_handler` would not return anyhow!(msg).
It's will return WrapperDiagnostics as it's Err. Then you can take
diagnostics from WrapperDiagnostics or get pretty string from it.

```diff
- pub fn try_with_handler<F, Ret>(cm: Lrc<SourceMap>, config: HandlerOpts, op:F) -> Result<Ret, anyhow::error>;
+ pub fn try_with_handler<F, Ret>(cm: Lrc<SourceMap>, config: HandlerOpts, op:F) -> Result<Ret, WrapperDiagnostics>;
```

```rust
/// A wrapper around a value that also contains a list of diagnostics.
/// It helps swc error system migrate to the new error reporting system.
pub struct WrapperDiagnostics {
    diagnostics: Vec<SwcDignostic>,
   ....
}

impl WrapperDiagnostics {
    pub fn diagnostics(&self) -> &[Diagnostic] {
        &self.diagnostics
    }

    pub fn to_pretty_error(&self) -> anyhow::Error {
        let error_msg = self.to_pretty_string();

        anyhow::anyhow!(error_msg)
    }


   pub fn to_pretty_string(&self) -> String {
      .... 
    }
}
```

### formatting

In SWC, the `Syntax Error` message formatting dependents anyhow. 
```rust
// input: Foo {}
// https://play.swc.rs/?version=1.11.13&code=H4sIAAAAAAAAA3PLz1eorgUAR6TWygYAAAA%3D&config=H4sIAAAAAAAAA1WPSw7DIAwF9zkF8rrbdtE79BAWdSIifrKJVBTl7iUE0maH3xsz8jooBbNoeKq1PMsQkYX4nEsi2Sf8lARIOxTNJia49XaWvRrRCtVoOxpIyBOluiX3hoMNQajjLXPGmzH%2FC3VwkUnkCu4o%2BsnSVTc0JbjwXmrZDkk50qF%2FwA%2FqsvNjMPLqm4kXGrYvhlQioBQBAAA%3D

  x Expected ';', '}' or <eof>
   ,-[input.js:1:1]
 1 | Foo {}
   : ^|^ ^
   :  `-- This is the expression part of an expression statement
   `----


Caused by:
    0: failed to process js file
    1: Syntax Error
```

So if we return `Vec<Diagnostic>` instead of `anyhow:Error`, The output
formatting would have a litter breaking change.
```diff
  x Expected ';', '}' or <eof>
   ,-[input.js:1:1]
 1 | Foo {}
   : ^|^ ^
   :  `-- This is the expression part of an expression statement
   `----

- Caused by:
-    0: failed to process js file
-    1: Syntax Error
+ x Syntax Error
```

# Related issue:

 - Closes #10192

---------

Co-authored-by: Donny/강동윤 <[email protected]>
kdy1 added a commit that referenced this pull request Apr 9, 2025
…10241)

# Description

SWC api `try_with_handler` only return anyhow!(msg) Err, it'a will lose many message about error stack, level ...,
To express more error message in swc, we can return `Vec<Diagnostic>` in `try_with_handler`;

try_with_handler_diagnostic return a `Result<Ret, Vec<Diagnostic>>`. Later user can using `diagnostic.code` to distinguish what kind error is.




- Added `miette` as a dependency for improved error handling and reporting.
- Updated diagnostic handling to utilize `miette` for pretty printing errors.
- Introduced a new `ThreadSafetyDiagnostics` to manage and store diagnostics.
- Refactored emitters to support the new diagnostic structure and ensure compatibility with `miette`.
- Enhanced error handling in various components to provide clearer and more informative error messages.



# BREAKING CHANGE:

The api `try_with_handler` would not return anyhow!(msg).
It's will return WrapperDiagnostics as it's Err. Then you can take
diagnostics from WrapperDiagnostics or get pretty string from it.

```diff
- pub fn try_with_handler<F, Ret>(cm: Lrc<SourceMap>, config: HandlerOpts, op:F) -> Result<Ret, anyhow::error>;
+ pub fn try_with_handler<F, Ret>(cm: Lrc<SourceMap>, config: HandlerOpts, op:F) -> Result<Ret, WrapperDiagnostics>;
```

```rust
/// A wrapper around a value that also contains a list of diagnostics.
/// It helps swc error system migrate to the new error reporting system.
pub struct WrapperDiagnostics {
    diagnostics: Vec<SwcDignostic>,
   ....
}

impl WrapperDiagnostics {
    pub fn diagnostics(&self) -> &[Diagnostic] {
        &self.diagnostics
    }

    pub fn to_pretty_error(&self) -> anyhow::Error {
        let error_msg = self.to_pretty_string();

        anyhow::anyhow!(error_msg)
    }


   pub fn to_pretty_string(&self) -> String {
      .... 
    }
}
```

### formatting

In SWC, the `Syntax Error` message formatting dependents anyhow. 
```rust
// input: Foo {}
// https://play.swc.rs/?version=1.11.13&code=H4sIAAAAAAAAA3PLz1eorgUAR6TWygYAAAA%3D&config=H4sIAAAAAAAAA1WPSw7DIAwF9zkF8rrbdtE79BAWdSIifrKJVBTl7iUE0maH3xsz8jooBbNoeKq1PMsQkYX4nEsi2Sf8lARIOxTNJia49XaWvRrRCtVoOxpIyBOluiX3hoMNQajjLXPGmzH%2FC3VwkUnkCu4o%2BsnSVTc0JbjwXmrZDkk50qF%2FwA%2FqsvNjMPLqm4kXGrYvhlQioBQBAAA%3D

  x Expected ';', '}' or <eof>
   ,-[input.js:1:1]
 1 | Foo {}
   : ^|^ ^
   :  `-- This is the expression part of an expression statement
   `----


Caused by:
    0: failed to process js file
    1: Syntax Error
```

So if we return `Vec<Diagnostic>` instead of `anyhow:Error`, The output
formatting would have a litter breaking change.
```diff
  x Expected ';', '}' or <eof>
   ,-[input.js:1:1]
 1 | Foo {}
   : ^|^ ^
   :  `-- This is the expression part of an expression statement
   `----

- Caused by:
-    0: failed to process js file
-    1: Syntax Error
+ x Syntax Error
```

# Related issue:

 - Closes #10192

---------

Co-authored-by: Donny/강동윤 <[email protected]>
@kdy1 kdy1 modified the milestones: Planned, v1.11.19 Apr 10, 2025
github-merge-queue bot pushed a commit to lynx-family/lynx-stack that referenced this pull request May 2, 2025
## Summary

<!-- Can you explain the reasoning behind implementing this change? What
problem or issue does this pull request resolve? -->

See:

- Rspack: web-infra-dev/rspack#10256
- Breaking changes:
    - `source_map_url`: swc-project/swc#10346
- `&mut DiagnosticBuilder<'_>`:
swc-project/swc#10241

<!-- It would be helpful if you could provide any relevant context, such
as GitHub issues or related discussions. -->

## Checklist

<!--- Check and mark with an "x" -->

- [ ] Tests updated (or not required).
- [ ] Documentation updated (or not required).
@swc-project swc-project locked as resolved and limited conversation to collaborators May 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants