1774866 - Support structured cloning of Error stacks

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task)

Description

[Tom S [:evilpie]]

Internally the stack of an Error object is represented using SavedFrames. These SavedFrames have an associated principal to make sure that we don't leak cross-origin stack frames or allow other unprivileged accesses. This is a problem because principals currently don't work well off-the-main-thread. We can "write" (clone) principals on Worker or Worklet threads, but we can only "read" and "write" principals on the main thread.

For example that means we can support sending Error stacks from a Worker to the main script, but not in reverse.

Comment 1

[Tom S [:evilpie]]

Attachment: Bug 1774866 - Support error stacks in main-thread structuredClone(). r?smaug

Comment 2

[Tom S [:evilpie]]

Attachment: WIP: Bug 1774866 - Support error stacks in Console functions (log etc.)

Depends on D149696

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3668b759a13
Support error stacks in main-thread structuredClone(). r=smaug

Comment 4

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34612 for changes under testing/web-platform/tests

Comment 5

[bszekely]

https://hg.mozilla.org/mozilla-central/rev/b3668b759a13

Comment 6

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 7

[Tom Schuster (MoCo)]

Attachment: Bug 1774866 - Support error stacks in window.postMessage. r?nika,smaug

Comment 8

[Tom S [:evilpie]]

Attachment: WIP: Bug 1774866 - Support error stacks in postMessage from Worker to main-thread.

Depends on D150625

Comment 9

[Tom S [:evilpie]]

Could you take a look at my patch in comment 8 and tell me if that approach makes any sense? We can only read principals on the main thread, so I am trying to only allow postMessaging when the target is the main thread.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Do we want to support history.pushState() ?

Comment 12

[Tom S [:evilpie]]

(In reply to Olli Pettay [:smaug] from comment #11)

Do we want to support history.pushState() ?

I don't think anything speaks against it. This seems to be a Window (main-thread) only API.

So far I have mostly been trying to just work on what I think is important.

Could you also look at the Worker patch, I would like to make some progress there.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ba46b75162c
Support error stacks in window.postMessage. r=smaug

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34831 for changes under testing/web-platform/tests

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1ba46b75162c

Comment 16

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[Hamish Willee]

Docs note:
My understanding is that this is to go into FF104. This is a tracking note so we remember to update the docs (in particular the release note). BCD will already include this information from the update in https://github.com/mdn/browser-compat-data/pull/17008

I also added dev-doc-needed so we can track this.

Comment 18

[Hamish Willee]

FF104 docs for this can be tracked in https://github.com/mdn/content/issues/18998

Tom, this is marked as "leave open" . Is there anything that will change about this feature?

The BCD already has this note:

"Version 104 additionally supports serialization of stack in window.postMessage() and structuredClone().

I think all we need to do is add a docs release note that says much the same thing.

Comment 19

[Tom S [:evilpie]]

Is there anything that will change about this feature?

We are going to add stack cloning support to other APIs like worker's postMessage.

For now the documentation looks good.

Comment 20

[Hamish Willee]

Thanks very much. Not removing dev-doc-needed because presumably we'll need to track those other additions.

Comment 22

[Tom S [:evilpie]]

Attachment: Bug 1774866 - Always allow cloning error stacks. r?nika!,sfink!,smaug!

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8732c83f62f8
Always allow cloning error stacks. r=nika,sfink,smaug

Comment 24

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/8732c83f62f8

Comment 25

[Ruth John]

MDN updates can be tracked here https://github.com/mdn/content/issues/23686

Comment 26

[Hamish Willee]

Support for structured cloning of native types in FF104 (Bugzilla #1556604 "Allow structured cloning of native error types") was added for serializing stack in window.postMessage() and structuredCloned(). This resulted in addition of compatibility notes as shown here: https://github.com/mdn/browser-compat-data/pull/17008

Comment 19 above says:

We are going to add stack cloning support to other APIs like worker's postMessage.

1. Can you confirm that this was what is delivered by this bug fix? Anything else?
2. My assumption is that this can just be added as another note to the compatibility data like the ones in the issue linked above (i.e. it is not that the implementation was partially spec compliant and is fully compliant now - since the spec just says "serialize interesting data").

Comment 27

[Tom S [:evilpie]]

Can you confirm that this was what is delivered by this bug fix? Anything else?

Yeah. Cloning native error stacks now works for everything that uses the structured clone algorithm. e.g. postMessage, structuredClone in Workers etc.

The only related missing thing is now bug 1777321.

Comment 28

[Hamish Willee]

Thanks very much. FYI only, I've updated the browser compatibility as indicated and added an MDN release note (see status here). I don't think anything else is needed.