JavaScriptCore: Fixes for exception/error handling when inside a SWIG wrapper function #180

Closed
wants to merge 4 commits into
from

Projects

None yet

2 participants

@ewmailing

This allows "sourceURL", "line", and "message" to be recovered from the exception object by the user so they can track down errors in their code.

Additionally, this makes sure error conditions return an actual object (JSValueMakeUndefined) instead of NULL to prevent possible crashes or weirdness.

The commit logs contain a lot more details.

Eric Wing added some commits May 20, 2014
Eric Wing JavaScriptCore: Fixed exception object so sourceURL (file name), line…
… (number), and message can be recovered.

The current implementation only returns an error string. But this is insufficient for debugging (what file and line number did it fail at?).

As documented here:
http://parmanoir.com/Taming_JavascriptCore_within_and_without_WebView
converting the JSValueRef of string to an JSObjectRef (via JSValueToObject) will trigger JSCore into filling the "sourceURL" and "line" properties into the object so they can be inspected by the caller.

Additionally, JavaScriptCore has a "message" property which contains the message string. JSCore doesn't seem to be filling this in for us automatically, unlike "sourceURL" and "line". So this patch also fills that property in too.

Thanks to Brian Barnes for the detailed information about "sourceURL", "line", and "message".

Below is an example (derived from Brian Barnes's information) on how you typically use/extract these exception details.

void script_exception_to_string(JSContextRef js_context,JSValueRef exception_value_ref,char* return_error_string, int return_error_string_max_length)
{
	JSObjectRef exception_object;
	JSValueRef value_ref;
	JSStringRef jsstring_property_name = NULL;
	JSValueRef temporary_exception = NULL;
	JSStringRef js_return_string = NULL;
	size_t bytes_needed;
	char* c_result_string = NULL;
	exception_object = JSValueToObject(js_context, exception_value_ref, NULL);
	// source and line numbers

	strcpy(return_error_string,"[");

	jsstring_property_name = JSStringCreateWithUTF8CString("sourceURL");
	value_ref = JSObjectGetProperty(js_context, exception_object, jsstring_property_name, &temporary_exception);
	JSStringRelease(jsstring_property_name);
	js_return_string = JSValueToStringCopy(js_context, value_ref, NULL);
	bytes_needed = JSStringGetMaximumUTF8CStringSize(js_return_string);
	c_result_string = (char*)calloc(bytes_needed, sizeof(char));
	JSStringGetUTF8CString(js_return_string, c_result_string, bytes_needed);
	SDL_Log("c_result_string: %s\n", c_result_string);
	JSStringRelease(js_return_string);
	strncat(return_error_string, c_result_string, return_error_string_max_length-1);
	free(c_result_string);

	strncat(return_error_string, ":", return_error_string_max_length-1);

	jsstring_property_name = JSStringCreateWithUTF8CString("line");
	value_ref = JSObjectGetProperty(js_context, exception_object, jsstring_property_name, &temporary_exception);
	JSStringRelease(jsstring_property_name);
	js_return_string = JSValueToStringCopy(js_context, value_ref, NULL);
	bytes_needed = JSStringGetMaximumUTF8CStringSize(js_return_string);
	c_result_string = (char*)calloc(bytes_needed, sizeof(char));
	JSStringGetUTF8CString(js_return_string, c_result_string, bytes_needed);
	SDL_Log("c_result_string: %s\n", c_result_string);
	JSStringRelease(js_return_string);
	strncat(return_error_string, c_result_string, return_error_string_max_length-1);
	//SDL_Log("c_result_string: %s\n", c_result_string);
	free(c_result_string);

	strncat(return_error_string, "]", return_error_string_max_length-1);

	/* get message */
	jsstring_property_name = JSStringCreateWithUTF8CString("message");
	value_ref = JSObjectGetProperty(js_context, exception_object, jsstring_property_name, &temporary_exception);
	JSStringRelease(jsstring_property_name);
	if(NULL == value_ref)
	{
		strncat(return_error_string, "Unknown Error", return_error_string_max_length-1);
	}
	else
	{
		js_return_string = JSValueToStringCopy(js_context, value_ref, NULL);
		bytes_needed = JSStringGetMaximumUTF8CStringSize(js_return_string);
		c_result_string = (char*)calloc(bytes_needed, sizeof(char));
		JSStringGetUTF8CString(js_return_string, c_result_string, bytes_needed);
		SDL_Log("c_result_string: %s\n", c_result_string);
		JSStringRelease(js_return_string);
		strncat(return_error_string, c_result_string, return_error_string_max_length-1);
		//SDL_Log("c_result_string: %s\n", c_result_string);
		free(c_result_string);
	}
}

To use:
if(js_exception)
{
	char return_error_string[256];
	script_exception_to_string(js_context, js_exception, return_error_string, 256);
	SDL_Log("Compile error is %s", return_error_string);
}
be938aa
Eric Wing JavaScriptCore: Returning NULL for wrapper functions that expect JSVa…
…lueRef may crash program.

According to this:
http://parmanoir.com/Taming_JavascriptCore_within_and_without_WebView
Returning NULL instead of an actual JSValueRef for a return value of a function could lead to crashes. I think I have seen related weirdness in the past when I failed to return a proper type to JSCore which resulted in very hard to understand behavior.

So this patch changes those return NULLs to return JSValueMakeUndefined().

I thought about JSObjectMakeError, but I don't fully understand the intent of the Error object and can't find any relevant real world examples of it being used. However, everybody seems to be using JSValueMakeUndefined().

This patch should be low impact since this is only triggered on an error condition.
8b3bd4f
Eric Wing JavaScriptCore: Reverted 2 of the JSValueMakeUndefined replacements b…
…ecause those functions are tied to JSObjectRef instead of JSValueRef. The C compiler will allow this, but C++ will reject the conversion.

It is unclear what the correct handling is for JavaScriptCore. (Nobody bothers to document this in JSCore.) Unlike our other problem where we incorrectly assume JSObjectRef when the functions want JSValueRef, this time Apple is demanding the JSObjectRef. Like our other problem, I assume it is unsafe to try to convert Undefined into a JSObjectRef.

So reverting to NULL seems like the safer bet for this specific case. Perhaps the other alternative is to return an exception object or an error object. But I would like to see JSCore document this before trying.
23e4547

I added one more commit. I saw that the compile would fail for C++ on constructors because they return JSObjectRef instead of JSValueRef. This last commit backs out the 2 specific JSObjectRef cases.

Eric Wing JavaScriptCore: Improved code that uses JSObjectMakeError instead of …
…JSValueToObject to create the exception object.

JSObjectMakeError automatically populates the "message" field, and possibly other fields I don't know about. This seems to be the most robust way to create an exception object.

Thanks to Brian Barnes again for the tip on JSObjectMakeError.
eac03c8
Contributor

Merged.

@oliver---- oliver---- closed this May 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment