Error handling for v8::ValueSerializer/Deserializer |
||
Issue descriptionWhen an error occurs during serialization/deserialization, V8's embedder (i.e., Blink) must be able to handle the errors. In general, Blink handles errors during serialization by throwing the DataCloneError DOMException, and errors during deserialization by producing null instead. V8 itself does not know about DOMException, so it must report error more generically for Blink. I see three options: 1. V8 throws no exception but does return Nothing<T>(), and Blink handles that by throwing DataCloneError. - what the code does today (out of sheer simplicity) - not flexible enough to report a more specific error message - slightly confusing because most APIs that return Maybe/MaybeLocal do throw an exception when they fail 2. V8 throws a specially marked exception (e.g. with a private symbol), and provides a public API for detecting such exceptions. Blink catches this and rethrows it as a DataCloneError, preserving the exception message. - slightly quirky, but works 3. V8's embedder provides a function which throws a suitable exception given its message (or the details needed to construct a message). - adds another virtual V8-to-embedder call, which seems undesirable (albeit only during exception throwing, which is not a fast path) WDYT?
,
Sep 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f72351f5400151d9a4b63da2be4049f4c8155e75 commit f72351f5400151d9a4b63da2be4049f4c8155e75 Author: jbroman <jbroman@chromium.org> Date: Fri Sep 02 15:16:26 2016 Throw exceptions for errors in v8::ValueSerializer. BUG= chromium:148757 , chromium:641964 Review-Url: https://codereview.chromium.org/2307603002 Cr-Commit-Position: refs/heads/master@{#39140} [modify] https://crrev.com/f72351f5400151d9a4b63da2be4049f4c8155e75/include/v8.h [modify] https://crrev.com/f72351f5400151d9a4b63da2be4049f4c8155e75/src/api.cc [modify] https://crrev.com/f72351f5400151d9a4b63da2be4049f4c8155e75/src/messages.h [modify] https://crrev.com/f72351f5400151d9a4b63da2be4049f4c8155e75/src/value-serializer.cc [modify] https://crrev.com/f72351f5400151d9a4b63da2be4049f4c8155e75/src/value-serializer.h
,
Sep 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/5a97e955204c4782d9727e8248b12739720232ba commit 5a97e955204c4782d9727e8248b12739720232ba Author: jbroman <jbroman@chromium.org> Date: Tue Sep 06 03:24:35 2016 Handle errors in v8::ValueDeserializer by throwing exceptions. This restores the contract that all API methods that return Maybe<T> or MaybeLocal<T> always throw an exception when they return nothing. Since v8::ValueDeserializer::ReadHeader can now throw exceptions, it needs a Local<Context> parameter so that it can set up execution state (entering the context, etc.). The old method has been marked for deprecation, but since this API is experimental I intend to remove it as soon as I've removed the use from Blink. value-serializer-unittest has been updated to expect an exception in all decode failure cases. BUG= chromium:148757 , chromium:641964 Review-Url: https://codereview.chromium.org/2308053002 Cr-Commit-Position: refs/heads/master@{#39188} [modify] https://crrev.com/5a97e955204c4782d9727e8248b12739720232ba/include/v8.h [modify] https://crrev.com/5a97e955204c4782d9727e8248b12739720232ba/src/api.cc [modify] https://crrev.com/5a97e955204c4782d9727e8248b12739720232ba/src/counters.h [modify] https://crrev.com/5a97e955204c4782d9727e8248b12739720232ba/src/messages.h [modify] https://crrev.com/5a97e955204c4782d9727e8248b12739720232ba/src/value-serializer.cc [modify] https://crrev.com/5a97e955204c4782d9727e8248b12739720232ba/test/unittests/value-serializer-unittest.cc
,
Sep 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1e7daa60cb845a89dd455f9d363afb6e929cfe1 commit a1e7daa60cb845a89dd455f9d363afb6e929cfe1 Author: jbroman <jbroman@chromium.org> Date: Wed Sep 07 01:07:42 2016 Provide v8::Context to v8::ValueDeserializer::ReadHeader. Since it can now throw exceptions, it should be explicitly given a context in which to do so. This allows the no-argument version to be removed from V8. BUG= chromium:148757 , chromium:641964 Review-Url: https://codereview.chromium.org/2318743002 Cr-Commit-Position: refs/heads/master@{#416810} [modify] https://crrev.com/a1e7daa60cb845a89dd455f9d363afb6e929cfe1/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
,
Jan 4 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by haraken@chromium.org
, Aug 29 2016