Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/unlucky-plants-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: don't define `error.message` if it's not configurable
22 changes: 18 additions & 4 deletions packages/svelte/src/internal/client/error-handling.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DEV } from 'esm-env';
import { FILENAME } from '../../constants.js';
import { is_firefox } from './dom/operations.js';
import { BOUNDARY_EFFECT, EFFECT_RAN } from './constants.js';
import { define_property } from '../shared/utils.js';
import { define_property, get_descriptor } from '../shared/utils.js';
import { active_effect } from './runtime.js';

/**
Expand Down Expand Up @@ -63,6 +63,11 @@ function adjust_error(error, effect) {
if (adjusted_errors.has(error)) return;
adjusted_errors.add(error);

const message_descriptor = get_descriptor(error, 'message');

// if the message was already changed and it's not configurable we can't change it
// or it will throw a different error swallowing the original one

var indent = is_firefox ? ' ' : '\t';
var component_stack = `\n${indent}in ${effect.fn?.name || '<unknown>'}`;
var context = effect.ctx;
Expand All @@ -72,9 +77,18 @@ function adjust_error(error, effect) {
context = context.p;
}

define_property(error, 'message', {
value: error.message + `\n${component_stack}\n`
});
if (!message_descriptor || message_descriptor.configurable) {
define_property(error, 'message', {
value: error.message + `\n${component_stack}\n`
});
} else {
// eslint-disable-next-line no-console
console.error(
"The following it's not a separate error: we usually modify the error message to show you this information but `message` was non configurable so we print them in a separate log.\n" +
error.message +
`\n${component_stack}\n`
);
Copy link
Member

Choose a reason for hiding this comment

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

This is ... a lot 😄 I would trim this down to just this, or just not log anything at all. I don't know how many people will have worse stacks as a result, but I doubt it's many

Suggested change
console.error(
"The following it's not a separate error: we usually modify the error message to show you this information but `message` was non configurable so we print them in a separate log.\n" +
error.message +
`\n${component_stack}\n`
);
console.error(
"[adjusted stack] " +
error.message +
`\n${component_stack}\n`
);

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I feared it would've been a lot but also wanted to make it clear it was not a separate error

}

if (error.stack) {
// Filter out internal modules
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { test } from '../../test';

export default test({
compileOptions: {
dev: true
},
error: 'test'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
class CustomError extends Error {
constructor() {
super();
Object.defineProperty(this, "message", {
value: "test"
});
}
}
throw new CustomError()
</script>