Bad "fn is not a function" error when methods are in the code

[josephjclark]

The error comes out of the runtime. It has nothing to do with common.fn - it's just trying to invoke an argument called fn. It's a confusing variable name, basically.

Maybe we should fix that, but the bigger story here is that the compiler needs to behave better in this case.

Or maybe the runtime does?

Basically because console.log isn't an operation, it doesn't return a function, and so it breaks the execution pipeline.

Options:

• Be more robust in the runtime - catch the error and the line and say "are you sure this is an Operation?"
• Catch the issue in the compiler - which is semantic analysis, not syntactic, so may not be possible or appropriate

[doc-han]

Awesome work on kit. @josephjclark

Be more robust in the runtime

wouldn't it be a bit more tricky to try catching the error? given that there are several other errors that bubble up to that point. eg. ReferenceError and other TypeError that might be from inside fn and not from runtime code.

How about just checking whether fn is a function in the runtime before executing it? Hence if it's not a func then an error is thrown. maybe InvalidOperation.

Still doesn't seem to solve the problem of it not being a function that returns state but at least makes sure that we have a function.
Also, runtime doesn't seem to have line information relative to the source code provided by a user.

---

Catch the issue in the compiler

Yup, semantic analysis may take a very long time to achieve. How about instead of waiting till runtime to catch the function error described above. Would it be better to catch this at compile time? because we reach here first.

Making sure that file.program.body contains only FunctionDeclaration | ExpressionStatement->expression->ArrowFunctionExpression | ExpressionStatement->expression->CallExpression a function or function call.
With this we can error at compile time with a line number and code snippet saying "are you sure this is an Operation?"

Assuming all the top level code are from imports.

If all top-level Nodes at file.program.body are from adaptor imports. Then after addUsedImports in the compiler, why not check those top-level Nodes that aren't from imports and then error them with "are you sure this is an Operation?"

eg.

import { fn } from "@openfn/language-common"

fn(state=> state) // assuming users always use defined funcs

// assuming no use does any of the below at the top-level
(state) => state; 
console.log("smth");
... // things that aren't from adaptors

What are your thoughts?

[josephjclark]

Hi @doc-han - are you looking for some interesting work in the compiler or runtime? I'm sure I can find a good ticket for you :)

[doc-han]

Alright, sounds cool. looking forward to the tickets.

Also, I'll like to know your thoughts on this one.

[josephjclark]

@doc-han I'll get back to you tomorrow - not at my desk today!

[josephjclark]

Hey @doc-han , it would be great to keep your eyes on this and one closely related problem next.

How about just checking whether fn is a function in the runtime before executing it? Hence if it's not a func then an error is thrown. maybe InvalidOperation.

Yes, happy with this. I think you have to check it's a function OR a promise 🤔 That should quickly become obvious when you check the code.

solve the problem of it not being a function that returns state

I've been meaning to address this forever - see #737

Ok, it's not a very good issue. But if any operation fails to return a state object (and this happens all the time actually!) we should console.log a really loud warning and set state to {} before going to the next expression.

in the compiler, why not check those top-level Nodes that aren't from imports and then error them with "are you sure this is an Operation?"

Yes, we could probably do this! But at the moment I'm not keen on a compiler fix because our main app, Lightning, doesn't actually show compiler logs. So the users who need it most wouldn't get the warning.

We'll leave the compiler alone for now - although if you're particularly interested in the compiler I do think I have some useful issues you could look at.