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

fix: error messages are getting lost #1263

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

mrgrain
Copy link
Contributor

@mrgrain mrgrain commented Aug 21, 2024

All errors encountered jsii used to print in the following order:

  1. Any logging output (might be empty)
  2. Full CLI help
  3. Actual error including stack trace

This has a few downsides:

  • logging output is hidden from view due to long cli help
  • cli help is not useful in most cases
  • stack trace is not relevant to solving the error

Instead we now print

  1. Any logging output
  2. CLI help only if explicitly requested by the error
  3. Just the error message with a nice red prefix

Also deletes some compiled js files that were incorrectly checked-in to the repo when it was migrated from aws/jsii.


Previously:
image
[...]
image

Now:
image


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Comment on lines +194 to +208
} catch (e: unknown) {
if (e instanceof utils.JsiiError) {
if (e.showHelp) {
console.log();
yargs.showHelp();
console.log();
}

const LOG = log4js.getLogger(utils.CLI_LOGGER);
LOG.error(e.message);

process.exitCode = -1;
} else {
throw e;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff for this file is weird. This is the new part + the code above is now wrapped in a try-catch.

@aws-cdk-automation aws-cdk-automation added this pull request to the merge queue Aug 21, 2024
Merged via the queue into main with commit 20e1570 Aug 21, 2024
78 checks passed
@aws-cdk-automation aws-cdk-automation deleted the mrgrain/fix/improve-error-output branch August 21, 2024 12:27
aws-cdk-automation pushed a commit that referenced this pull request Aug 21, 2024
All errors encountered jsii used to print in the following order:
1. Any logging output (might be empty)
2. Full CLI help
3. Actual error including stack trace

This has a few downsides:
- logging output is hidden from view due to long cli help
- cli help is not useful in most cases
- stack trace is not relevant to solving the error

Instead we now print
1. Any logging output
2. CLI help only if explicitly requested by the error
3. Just the error message with a nice red prefix

---

Also deletes some compiled js files that were incorrectly checked-in to
the repo when it was migrated from `aws/jsii`.

---

Previously:
<img width="1167" alt="image"
src="https://github.com/user-attachments/assets/2f44ae43-b1de-4883-a868-296968910dfa">
[...]
<img width="1063" alt="image"
src="https://github.com/user-attachments/assets/c460aeab-1e64-4ce1-afb0-5d6053c2f398">

Now:
<img width="1174" alt="image"
src="https://github.com/user-attachments/assets/37da4d64-dd2a-4b88-b7a0-fcbb8511baa3">

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

(cherry picked from commit 20e1570)
aws-cdk-automation pushed a commit that referenced this pull request Aug 21, 2024
All errors encountered jsii used to print in the following order:
1. Any logging output (might be empty)
2. Full CLI help
3. Actual error including stack trace

This has a few downsides:
- logging output is hidden from view due to long cli help
- cli help is not useful in most cases
- stack trace is not relevant to solving the error

Instead we now print
1. Any logging output
2. CLI help only if explicitly requested by the error
3. Just the error message with a nice red prefix

---

Also deletes some compiled js files that were incorrectly checked-in to
the repo when it was migrated from `aws/jsii`.

---

Previously:
<img width="1167" alt="image"
src="https://github.com/user-attachments/assets/2f44ae43-b1de-4883-a868-296968910dfa">
[...]
<img width="1063" alt="image"
src="https://github.com/user-attachments/assets/c460aeab-1e64-4ce1-afb0-5d6053c2f398">

Now:
<img width="1174" alt="image"
src="https://github.com/user-attachments/assets/37da4d64-dd2a-4b88-b7a0-fcbb8511baa3">

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

(cherry picked from commit 20e1570)
@aws-cdk-automation
Copy link
Collaborator

💚 All backports created successfully

Status Branch Result
maintenance/v5.3
maintenance/v5.4

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2024
# Backport

This will backport the following commits from `main` to
`maintenance/v5.3`:
- [fix: error messages are getting lost
(#1263)](#1263)

<!--- Backport version: 9.5.1 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

Co-authored-by: Momo Kornher <kornherm@amazon.co.uk>
github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2024
# Backport

This will backport the following commits from `main` to
`maintenance/v5.4`:
- [fix: error messages are getting lost
(#1263)](#1263)

<!--- Backport version: 9.5.1 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

Co-authored-by: Momo Kornher <kornherm@amazon.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants