onFormSubmit e.values empty

[Roy Brophy]

I have a spreadsheet linked to a form and do some processing when the onFormSubmit installable trigger fires. 

Most of the time it works just fine, but occasionally the e.values are empty, even though the correct data has been written to the spreadsheet.

What might I be doing wrong?

Thanks,

Roy

[Alan Wells]

e.values should never be empty if your code is immediately referencing the object. 

I recommend getting the values out of the event object immediately, and then don't use the event object again in your code.  And definitely don't try to pass the event object to another function.  Get everything you need out of the event object, and store the values in something other than the event object.

One potential problem happens when users make multiple Form submissions in rapid succession, then the On Form Submit function could start running a second time before the first instance of the function has completed.  Forms with low usage, probably never have this problem, but if you have a Form that multiple people are filling out at the same time of day, it can happen.  In that case you should implement LockService.

[Roy Brophy]

AJ Many thanks for your help. Apologies for my first post being a little light on detail. It was 1:25am and I had been debugging this all day!

Yes - my code does grab e.values and store it, and not use it again Also this form/sheet has not been deployed yet - so not a problem with multiple users. I have a lock in place for when it's deployed.

However - I have written some test code in an effort to understand exactly what is going on. With some surprising results that has opened up a can of worms.

I have added a Test sheet to my spreadsheet. I have commented out all of the code in the onFormSubmit and added the following code to it. It basically stores the data that's been written to the Form responses 1 sheet, and the e.values to consecutive rows on the test sheet.

function onFormSubmit(e) {

  // Grab the e.values as soon as they arrive

  var submittedDataLocal = [];

  var submittedDataLocal = e.values;

  

  // set up the response and test sheet objects and find the last used rows in each

  var shtResp = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Form responses 1");

  var shtTest = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Test");

  var LastRowResp = shtResp.getLastRow();

  var LastRowTest = shtTest.getLastRow();

  

  // get the first seven cells of the row that's just been submitted from the response sheet

  var tmpRespLine = shtResp.getRange(LastRowResp,1,1,7).getValues();

  // write it to the next available line in the test sheet and make it pink

  shtTest.getRange(LastRowTest+1,1,1,7).setValues(tmpRespLine);

  shtTest.getRange(LastRowTest+1,1,1,7).setBackground("pink");

  

  // Now write the values found in e.values

  var submittedDataLocallength = submittedDataLocal.length;

  shtTest.getRange(LastRowTest+2,1,1,submittedDataLocallength).setValues([submittedDataLocal]);

 I ran the form once - this is what ended up in the Test sheet...

So - it seems that onFormSubmit trigger is firing twice. The second time with the e.values missing!

Having Googled this I see there are a number of post on the subject - I will follow up on these later today.

Misery :-(

Thanks,

Roy

[Alan Wells]

I don't see anything obviously wrong with your code.  I just realized, that I'm getting the same behavior in one of my sheets, but I haven't bothered to research it yet.  I've been getting an extra blank row with just the date in column A in my error log sheet.  I thought it was strange, but assumed that it was my code.  Now I'm wondering if it's an Apps Script bug.

[Clark Lind]

Instead of getting the last row (which is a dynamic value and uncertain), try using getActiveRange. That should always point to the data that triggered the onFormSubmit I would think. I would also just use appendRow to the test sheet instead of trying to calculate it.

On Saturday, May 25, 2019 at 5:43:09 AM UTC-4, Roy Brophy wrote:

[Roy Brophy]

OK - It seems that this is a known bug - but not that common. I can see a number of questions about onFormSubmit firing multiple times on the web. In testing today I have seen a maximum of three triggers when there should have only been one. One person reported six!

Another person reported that he copied the form/spreadsheet to make a new system and that fixed his problems. That didn't work for me.

There doesn't appear to be a fix. So I intend to add code to detect spurious triggers. The good news is that when the spurious trigger goes off, it is not adding anything to the spreadsheet - just triggers onFormSubmit and passes a Timestamp and nothing else in e.

My first question in the form is required. So I will look for an empty first field in e.values, and return from onFormSubmit after first logging that I have seen an unexpected trigger, and also send an email to the admin (me). That way I can keep track of it.

The code is shown below. I have also shown the Lock code I'm using - I'm not very experienced with this and would appreciate any advice.

BTW cwind - thanks for your thought about last row being uncertain - my code was indeed just picking up the row from the last correct triggering. However, when the incorrect trigger occurs, it is not writing to the spreadsheet, so I'm not sure what getActiveRange is going to return.

function onFormSubmit(e) {

  // Grab the e.values as soon as they arrive

 var submittedDataLocal = [];

 var submittedDataLocal = e.values;

  if (e.values[1] == "") {

   // This must be a spuriouse trigger

   // set up the sheet objects and find the last used row

   var shtTest = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Spurious onFormSubmit Triggers");

   var LastRowTest = shtTest.getLastRow();

   // Now write the values found in e.values - nothing except for the timestamp

   var submittedDataLocallength = submittedDataLocal.length;

   var submittedDataLocallength = submittedDataLocal.length;

   shtTest.getRange(LastRowTest+1,1,1,submittedDataLocallength).setValues([submittedDataLocal]);

   return;

 }  

 

 // Set lock so that another onFormSubmit can't fire until we are finished

 // Log the time it takes to get a lock

  var lock = LockService.getScriptLock();

 var success = lock.tryLock(30000);

 var nowTimeStamp = new Date();

 if (!success) {

   Logger.log('Could not obtain lock after 30 seconds.');

   UpdateHistorySheet(nowTimeStamp,"onSubmit","Unable to lock","","", "","");

   sendAdminEmail("LockTimeout ", 0, submittedDataLocal[0], "");

 } else {

   UpdateHistorySheet(nowTimeStamp,"onSubmit","Time to get lock",nowTimeStamp - startTime ,"", "","");

 }

  // processing here...

  //Tidy up...

 SpreadsheetApp.flush();

 lock.releaseLock();

)

[Roy Brophy]

OK - I re-organised the code so that the Lock is right up front and also to remove a buried return statement.

function onFormSubmit(e) {

  

  // Set lock so that another onFormSubmit can't fire until we are finished

  var lock = LockService.getScriptLock();

  var success = lock.tryLock(30000);

  var nowTimeStamp = new Date();

  if (!success) {

    // Could not obtain lock after 30 seconds

    UpdateHistorySheet(nowTimeStamp,"onSubmit","Unable to lock","","", "","");

    sendAdminEmail("LockTimeout ", 0, e.values[0], "");

  } else {

    // Grab the e.values as soon as they arrive

    var submittedDataLocal = [];

    var submittedDataLocal = e.values;

  

    if (e.values[1] == "") {

      // This must be a spurious trigger - log it in spreadsheet and send an Admin email

      // No further processing done

      // set up the test sheet object and find the last used row

      var shtSoT = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Spurious onFormSubmit Triggers");

      var LastRowSOT = shtSoT.getLastRow();

      // Now write the values found in e.values - that will be nothing except for the timestamp

      var submittedDataLocallength = submittedDataLocal.length;

      shtSoT.getRange(LastRowSOT+1,1,1,submittedDataLocallength).setValues([submittedDataLocal]);

      sendAdminEmail("SpuriousTrigger", 0, submittedDataLocal[0], "");

    } else {

      

      // Now we can process the data

      

    }    

  }

  

  //Tidy up...

  SpreadsheetApp.flush();

  lock.releaseLock();

 

} // End of onFormSubmit

[Alan Wells]

I would use a try/catch and release the lock in the "catch' because if there is an error, and the lock doesn't get released then you'll have a problem.  You don't want subsequence instances of the code to wait for the time-out to release the original lock.

[Roy Brophy]

Oh dear - red face here.... I have to admit that I have never used Try/Catch... 

And I have to deploy this tomorrow to 140 people :-(

I'll go away and do some reading - I feel an all-nighter coming on - again.

Any more detailed pointers would be much appreciated...

Thanks,

Roy

[Roy Brophy]

So is this what I need to do?

function onFormSubmit(e) {

  

  // Set lock so that another onFormSubmit can't fire until we are finished

  var lock = LockService.getScriptLock();

  var success = lock.tryLock(30000);

  var nowTimeStamp = new Date();

  if (!success) {

    // Could not obtain lock after 30 seconds

    UpdateHistorySheet(nowTimeStamp,"onSubmit","Unable to lock","","", "","");

    sendAdminEmail("LockTimeout ", 0, e.values[0], "");

  } else {

    // Grab the e.values as soon as they arrive

    var submittedDataLocal = [];

    var submittedDataLocal = e.values;

  

    if (e.values[1] == "") {

      // This must be a spurious trigger - log it in spreadsheet and send an Admin email

      // No further processing done

      // set up the test sheet object and find the last used row

      var shtSoT = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Spurious onFormSubmit Triggers");

      var LastRowSOT = shtSoT.getLastRow();

      // Now write the values found in e.values - that will be nothing except for the timestamp

      var submittedDataLocallength = submittedDataLocal.length;

      shtSoT.getRange(LastRowSOT+1,1,1,submittedDataLocallength).setValues([submittedDataLocal]);

      sendAdminEmail("SpuriousTrigger", 0, submittedDataLocal[0], "");

    } else {

      try {

      

        // Now we can process the data

           

      } catch (catchError) {

        // do some logging about the error 

        //Tidy up...

        SpreadsheetApp.flush();

        lock.releaseLock();

        return;

[Alan Wells]

 As a general rule, I wrap everything in the entire function in a try/catch - Why?  To cover every possible error.  Then if there are certain lines of code that I want to target, I'll have another try/catch inside of the outer try/catch.   Why would you want to target specific lines? 

* Do something else if there is an error - try to avoid a total failure

* Retry in hopes that a subsequent attempt will work

* Provide feedback to yourself about specific lines of code

It's a good idea in general to try to minimize the negative affects of errors.  So, using try/catch can sometimes, reduce the amount of damage done.  For example, there may be errors that can be ignored, and still produce some kind of end result for the user instead of a complete stop in the processing.
The error object has a message and the "stack."  The stack will tell you the exact line that the code failed on, and all the functions that were called in the program flow.

try{


}catch(e){
  myErrorHandlingFunction_(e);//Call my central error handling function

}

function myErrorHandlingFunction_(e) {
  var theStack;
  //What do you want to do every time that there is an error?
  //Send yourself an email?
  //Log the error to a spreadsheet that you own?
  //Send the error to stackdriver?  - console.log()
  //Make a POST request to an Apps Script Web App that you own?  That in turn sends you an email.
 
  theStack = e.stack;//Get the stack

}

[Roy Brophy]

Oh - I see - that's nice....

So my code will look like this -  

function onFormSubmit(e) {

  

  try {

    // Set lock so that another onFormSubmit can't fire until we are finished

    var lock = LockService.getScriptLock();

    var success = lock.tryLock(30000);

    var nowTimeStamp = new Date();

    if (!success) {

      // Could not obtain lock after 30 seconds

      UpdateHistorySheet(nowTimeStamp,"onSubmit","Unable to lock","","", "","");

      sendAdminEmail("LockTimeout ", 0, e.values[0], "");

    } else {

      // Grab the e.values as soon as they arrive

      var submittedDataLocal = [];

      var submittedDataLocal = e.values;

  

      if (e.values[1] == "") {

        // This must be a spurious trigger - log it in spreadsheet and send an Admin email

        // No further processing done

        // set up the test sheet object and find the last used row

        var shtSoT = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Spurious onFormSubmit Triggers");

        var LastRowSOT = shtSoT.getLastRow();

        // Now write the values found in e.values - that will be nothing except for the timestamp

        var submittedDataLocallength = submittedDataLocal.length;

        shtSoT.getRange(LastRowSOT+1,1,1,submittedDataLocallength).setValues([submittedDataLocal]);

        sendAdminEmail("SpuriousTrigger", 0, submittedDataLocal[0], "");

      } else {

      

        // Now we can process the data

           

      }    

    }

  } catch (catchError) {

    // call error logging function 

[Alan Wells]

I don't assign lock service to a variable.  That's because, if something goes wrong, then the variable for lock service may not be valid.  I'd make it look like this:

function onFormSubmit(e) {

  

  try {

    if (!LockService.getScriptLock().tryLock(2000)) {//Try to get a lock and if not successful it returns false
    Utilities.sleep(1000);//Wait x seconds
    if (!LockService.getScriptLock().tryLock(2000)) {//Try to get a lock and if not successful it returns false
      //Decide what you want to do - do nothing / let it run - There is a risk but maybe the code will be successful - or stop the process, and notify someone that there is a problem

    }
  }

    var nowTimeStamp = new Date();

    if (!success) {

      // Could not obtain lock after 30 seconds

      UpdateHistorySheet(nowTimeStamp,"onSubmit","Unable to lock","","", "","");

      sendAdminEmail("LockTimeout ", 0, e.values[0], "");

    } else {

      // Grab the e.values as soon as they arrive

      var submittedDataLocal = [];

      var submittedDataLocal = e.values;

  

      if (e.values[1] == "") {

        // This must be a spurious trigger - log it in spreadsheet and send an Admin email

        // No further processing done

        // set up the test sheet object and find the last used row

        var shtSoT = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Spurious onFormSubmit Triggers");

        var LastRowSOT = shtSoT.getLastRow();

        // Now write the values found in e.values - that will be nothing except for the timestamp

        var submittedDataLocallength = submittedDataLocal.length;

        shtSoT.getRange(LastRowSOT+1,1,1,submittedDataLocallength).setValues([submittedDataLocal]);

        sendAdminEmail("SpuriousTrigger", 0, submittedDataLocal[0], "");

      } else {

      

        // Now we can process the data

           

      }    

    }

  //Tidy up...

  SpreadsheetApp.flush();

  LockService.getScriptLock().releaseLock();

 

  } catch (errorObject) {

    LockService.getScriptLock().releaseLock();//Make sure the lock gets released if there is an error





    // call error logging function 

  }

} // End of onFormSubmit

[Roy Brophy]

Alan, thank you very much for your excellent help. 

I will do a little more reading and implement how you suggest.