Skip to content

Conversation

@rarkins
Copy link
Contributor

@rarkins rarkins commented May 28, 2025

Changes

Adds new typed logging for fatal messages

Context

I would like to consolidate and strongly type all warn, error and fatal messages

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@rarkins rarkins requested review from secustor and viceice May 28, 2025 05:35
return problems.clearProblems();
}

export function logFatal<C extends FatalCode, A extends AdditionalFieldsMap[C]>(
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 wasn't able to make this fit into logger.fatal() but I'm open to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This type came from ChatGPT. I tried using the Exact from type-fest instead, but it wasn't the same

Copy link
Member

@secustor secustor left a comment

Choose a reason for hiding this comment

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

I'm for this, but maybe there should a default handler for errorMessage, if error has been provided as a field.

So that errorMessage: err.message is the default, but can be overwritten.

@rarkins
Copy link
Contributor Author

rarkins commented May 28, 2025

So make err:Error an optional field for all?

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

failed CI checks

export async function generateErrors(dist: string): Promise<void> {
const errorFile = 'errors.md';
let errorFileContents = await readFile(`docs/usage/${errorFile}`);
const errorTypes = ['fatal'] as const;
Copy link
Member

Choose a reason for hiding this comment

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

what about the error type? 🤔

const errorTypes = ['fatal'] as const;
for (const errorType of errorTypes) {
const capitalizedType =
errorType.charAt(0).toUpperCase() + errorType.slice(1);
Copy link
Member

Choose a reason for hiding this comment

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

don't we have a util function already for this? 🤔

@rarkins rarkins added the auto:close-inactive-pr Inactive PR should be closed label Aug 16, 2025
@github-actions
Copy link
Contributor

Hi there,

We're closing this PR due to inactivity, but we are happy for you, or others, to finish the PR if time allows.

Thanks, the Renovate team

@github-actions github-actions bot closed this Aug 16, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

auto:close-inactive-pr Inactive PR should be closed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants