When an error is thrown in a chained native promise the printed stack is insufficient #815
Conversation
| @@ -63,7 +63,7 @@ void reportFatalErrorBeforeShutdown(ExecState* execState, Exception* exception, | |||
| std::cerr << "Native stack trace:\n"; | |||
| WTFReportBacktrace(); | |||
|
|
|||
| std::cerr << "JavaScript stack trace:\n"; | |||
There was a problem hiding this comment.
We'd better use NSLog instead of std::cerr because right now stderr lines are not added to the device logs.
There was a problem hiding this comment.
I tend to agree with that but we need to see if we want to swap all std:cerr calls with NSLog.
src/NativeScript/JSErrors.mm
Outdated
| @@ -73,6 +73,34 @@ void reportFatalErrorBeforeShutdown(ExecState* execState, Exception* exception, | |||
| } | |||
| std::cerr << "\n"; | |||
| } | |||
|
|
|||
| //OBSERVATION: | |||
There was a problem hiding this comment.
I would like to see this observation validated.
We can leave it as technical debt (although I really don't like this option) but we must know what is the reason for this behavior in JSC.
In my mind this is an issue that was there for at least an year. It can't be so critical that we would like to merge it immediately without knowing what is the specific reason for this behavior.
There was a problem hiding this comment.
@zahhak - in terms of behaviour the observation is validated. The open question here is whether this is an issue with JSC or an issue with our promise-based implementation for module resolution. I agree that we can spend some more time debugging here and decide if we want to release this as a patch or talk to Apple about it and do it afterwards.
|
There is an update to this fix which includes a change in the JavaScriptCore logic that we rely on: The failing PR build needs the WebKit PR to be approved and merged to pass. |
ginev
force-pushed
the
ginev/promise-error-fix
branch
from
ae0ea25
to
75b9c78
Compare
|
This PR partially addresses #565 as well. To completely fix it we need to also log using NSLog instead of writing to stderr. |
Discussion: #807