Throw error in calling plugin if call origin is a native

[rhelgeby]

I'm aware of both ThrowError and ThrowNativeError, but I need a universal function that will throw a native error if the call origin is from a native that I've implemented.

The reason for this is to keep the code that validates and throws errors close to the logic, but I want to avoid duplicated validation code in the native callback and internal code.

If the caller is another plugin, I simply want the error to bubble up to that plugin. If I called the function myself from within the plugin (from anything else than a native callback), it's my fault if something is not valid and my plugin should get the error.

Is this possible?

[Dr. Greg House]

Can't you safely verify the params first?

[rhelgeby]

I do, but third party plugins still may pass invalid parameters to my native. I don't want do duplicate validation logic in the native and in my "service" functions.

[Powerlord]

Unfortunately, I'm not aware of one. I've done tests in the past and ThrowNativeError will bomb out with an error (something like "not called from a native") if you call it from something other than a native.

[rhelgeby]

Yes, I discovered that myself. A combination would be useful, where it would just fall back to ThrowError if it's not in a native call.

[Dr. Greg House]

I do, but third party plugins still may pass invalid parameters to my native. I don't want do duplicate validation logic in the native and in my "service" functions.

I don't understand. You should do the verification inside the native. Not inside the plugin that calls it.

[rhelgeby]

No, that's not how I do it. The native simply delegates work to my "service" layer that will do it's own validation. If I add validation logic in the native handler, it will be duplicated.

[Greyscale]

Validation only need be done in the external API (because it's not guaranteed that your consumers will provide valid inputs). The internal implementation shouldn't need validation as all its usages are known.

Oh and hey how's it goin? :P

[Powerlord]

Validation only need be done in the external API (because it's not guaranteed that your consumers will provide valid inputs). The internal implementation shouldn't need validation as all its usages are known.

Oh and hey how's it goin? :P

You should probably also do validation in any stocks you plan on distributing to the public.

[rhelgeby]

I should have provided some examples earlier. Here's a real case:

Consider this native handler:
/**
* Creates a new module. The new module is bound to the calling plugin.
*
* Warning: You must call ZM_DeleteModule when the module is no longer in use,
* such as in OnPluginEnd. Otherwise the module manager will never know
* when the module is removed.
*
* Note: A plugin is only allowed to create one module. The module will remain
* registered until the plugin is unloaded.
*
* @param name Unique module name. Used by lookup functions.
*
* @return Module ID.
* @error Plugin has already created a module, name is already in
* use or name is empty.
*/
//native ZMModule:ZM_CreateModule(const String:name[]);
public API_CreateModule(Handle:plugin, numParams)
{
new String:name[MODULE_STRING_LEN];
GetNativeString(1, name, sizeof(name));

if (!AssertPluginHasNoModule(plugin)
|| !AssertModuleNameNotExists(name))
{
return 0;
}

new ZMModule:module = AddModule(plugin, name);
return _:module;
}

Look at how clean it is. I do as little as possible there. I'm just fetching parameters and delegating the work to another function. The asserts should probably be moved down to the service layer.

Here's AddModule:
ZMModule:AddModule(Handle:ownerPlugin, const String:moduleName[])
{
new ZMModule:module = CreateModule(ownerPlugin, moduleName);

AddModuleToList(module);
AddModuleToIndex(module);

return module;
}

AddModule belongs to the service layer. It doesn't create the module itself, but it adds it to an index so the plugin know which modules are available.

And here's CreateModule:
ZMModule:CreateModule(Handle:plugin, const String:name[])
{
if (plugin == INVALID_HANDLE)
{
ThrowNativeError(SP_ERROR_ABORTED, "Plugin not specified.");
}

if (PluginHasModule(plugin))
{
ThrowNativeError(SP_ERROR_ABORTED, "A module for this plugin already exists. Only one module per plugin is allowed.");
}

if (strlen(name) == 0)
{
ThrowNativeError(SP_ERROR_ABORTED, "Name is empty.");
}

InitModuleType();

new Object:module = ObjLib_CreateObject(ModuleType);
ObjLib_SetHandle(module, "plugin", plugin);
ObjLib_SetString(module, "name", name);

LogMessage("Created module: \"%s\" (%x)", name, module);

return ZMModule:module;
}

CreateModule belongs to the domain layer. This layer should be as isolated as possible and it do its own validation. This architecture is based on a rich domain model where domain objects (modules in this case) are smart and doesn't just contain data.

This is where my problem is, since it's used by natives I'm forced to use ThrowNativeError. What I'm looking for is a combination of ThrowError and ThrowNativeError. I don't see any technical limitations of this, since SourceMod tells me if the call does not come from a native handler. SourceMod could instead just fall back to ThrowError, or provide an additional native that does so.

Here's the complete code for those who are interested: https://github.com/rhelgeby/sm-zombiereloaded

[asherkin]

ThrowError and ThrowNativeError are for very different purposes, it doesn't make sense to muddle them together.

I your example, I think your goal of doing as little as possible in the native handler is the problem - that's where public API validation belongs (and you have some there, but all 3 TNE checks from CreateModule should be there). Once it gets into your service layer, you should be able to assume that the inputs are correct (as it's internal API, where checking is just pointless overhead).

[rhelgeby]

That would work too, but I don't agree with this approach.

The native layer can't (or shouldn't) know when input is invalid. However, it can of course know that something may go wrong, just not how to verify it itself. That would expose internals of the domain layer, which should be isolated. It also makes it difficult to keep the code in one place (some in natives, some in services and the rest in the domain).

The domain layer should (in my opinion) never accept invalid data, and should throw an error if so. This way I can make sure that my domain objects are always valid, even if there's a bug somewhere that would otherwise insert invalid data. In short, I don't trust the layer above me.

I can solve this myself by using a global boolean variable that I set when entering a native call and reset when leaving (or on an error). I would need to use wrappers for throwing errors, so this solution is dirty and not optimal.

[Powerlord]

I can solve this myself by using a global boolean variable that I set when entering a native call and reset when leaving (or on an error). I would need to use wrappers for throwing errors, so this solution is dirty and not optimal.

Or you could add an optional argument that, if set to true, means it's being called from a native.

[Greyscale]

Why do you need to validate your internal methods? I think that's the root cause of your issue here. If you just move all validation to the outer API then problem solved. If you want to do internal validation then, conceptually speaking, it's a different type of validation that you'll need to duplicate if you feel you need it. Otherwise your stuck with hacky solutions like the one you proposed.

[rhelgeby]

It's about creating robust isolated code, and to protect myself from making mistakes. Think like exceptions bubbling up the layers.

The only thing I'm missing is something to throw an error in the appropriate place.

[friagram]

There is no point to throw an error in your own api. You know how it works, and it works under strict set of assumptions. If a user passes something stupid to it, then it's expected something will go wrong.

If you want it to be foolproof, you need to either do what powerlord said, and make an arg to skip validation because its internal, or just always validate anyway.

[rhelgeby]

Yes, foolproof. That's the word I've been looking for. I'm trying to stick to a strong principle about creating robust code. And certainly not making assumptions. Even though I know how my code works, I will still make silly mistakes that will propagate through the code if I don't stop it early. This will help me while debugging.

The funny thing is that it's a tiny technical limitation in SourceMod that prevents me from doing it this way.

That's ok. I have another idea. I can use error codes in return values (or an error output parameter). I guess I've been trying code like SourcePawn have exceptiosn, but it doesn't. Error codes will work too.

Thanks for the feedback so far.