fix(cache): make dummy cache error ignorable

[vicb]

All "dummy" cache methods throw by design.
This PR makes the corresponding errors ignorable so that the logs are less verbose and always use the DEBUG level.

[changeset-bot]

🦋 Changeset detected

Latest commit: 920a7fd

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

This PR includes changesets to release 3 packages

Name	Type
@opennextjs/aws	Patch
app-pages-router	Patch
app-router	Patch

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

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	22.33%	1676 / 7503
🔵	Statements	22.33%	1676 / 7503
🔵	Functions	55.05%	98 / 178
🔵	Branches	69.41%	413 / 595

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/open-next/src/adapters/cache.ts	77.13%	80.89%	85.71%	77.13%	112-113, 174-176, 179-187, 197, 242-253, 280, 409-418, 422, 433-478
packages/open-next/src/overrides/incrementalCache/dummy.ts	0%	0%	0%	0%	1-17
packages/open-next/src/utils/error.ts	25%	20%	25%	25%	13-20, 32-39, 43-50

Generated in workflow #774 for commit 920a7fd by the Vitest Coverage Report Action

[pkg-pr-new]

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/aws@641

commit: 920a7fd

[conico974]

We may want to introduce another env variable OPEN_NEXT_ERROR_LOG_LEVEL to decide if we log the IgnorableError even in debug.
All of this would be decided inside the error function
By default it would only log level 1 error and above

[conico974]

Do we want to do this here, error should already be handling this

opennextjs-aws/packages/open-next/src/adapters/logger.ts

Lines 43 to 67 in 4a500ee

	export function error(...args: any[]) {
	// we try to catch errors from the aws-sdk client and downplay some of them
	if (
	args.some((arg: AwsSdkClientCommandErrorLog) => isDownplayedErrorLog(arg))
	) {
	debug(...args);
	} else if (args.some((arg) => arg.__openNextInternal)) {
	// In case of an internal error, we log it with the appropriate log level
	const error = args.find(
	(arg) => arg.__openNextInternal,
	) as BaseOpenNextError;
	if (error.logLevel === 0) {
	debug(...args);
	return;
	}
	if (error.logLevel === 1) {
	warn(...args);
	return;
	}
	console.error(...args);
	return;
	} else {
	console.error(...args);
	}
	}

Having only the error function deciding which error to log is probably better (and easier to read)

[vicb]

Oh I better understand what you meant now (I didn't noticed the handling of internal errors in the logger).

Still I find having IgnorableErrors and OPEN_NEXT_ERROR_LOG_LEVEL a bit overkill.

Let me think more about this.

[conico974]

I think IgnorableError alone are enough in most cases, debug mode should never be used outside of when you debug so you should never see such errors in production.
The OPEN_NEXT_ERROR_LOG_LEVEL has 1 main use imo, it's for adding the ability to mask RecoverableError logs, but it could also be used to mask IgnorableError in debug mode, which could be useful for us when testing something that has nothing to do with these errors

[vicb]

See #649