SalesOrder.Save() exception

[Mike.Sheen]

If the data reader is instantiated *by* a 'using' block, then closing the statement block will close the DataReader. By definition. Some refactoring tools can actually recommend losing the .Close().

But it's not.

The data reader is not instantiated *by* a using block in this case, but the SQLCommand is.

That might be ok, I'm just not used to accessing a DataReader after the SQLCommand is disposed of like that. I'm not saying that's causing the issue - without me being able to follow some steps to make the error happen I'm just pointing out something which smelled to me.

[Mark Shipman]

Having imported your plugin... what do I do now to reproduce the error?

I'm new to Jiwa, so please excuse the clumsy description.

1. System Settings :: Forms - add a form for the plugin.
2. System Settings :: Menu Configuration :: Menu Maintenance - add a menu item (XML export attached).
3. System Settings :: Staff Configuration :: User Group Maintenance - add user group (XML export attached).

The form I added to a folder called "Sales" (copied from the Magento menu item). Running the plugin is Sales :: (Menu item added in step 2 above)

There are two buttons. The left-hand one is the IMPORT button and, when clicked, will pop up a few message-box messages.

When it finishes the UI will become unbusy and two files are written to the users Roaming AppData directory in a sub-directory AERP\Logs. On my machine, this is C:\Users\mtshi\AppData\Roaming\AERP\Logs.

One of the files is named YYYMMDD.txt where YYYYMMDD is today's date. An example is attached showing the exception. If you look in your log file, you should see the same "New transaction..." exception.

[Mark Shipman]

At a glance the first thing which I found unfamiliar to me was how the method FirstSalesOrderUsingReferenceReader returns a SQL data reader inside a using statement for the SQLCommand. I'd change that method to close the reader before exiting.

The bold is good

The using statement has a singular purpose. it provides a block statement to which the variable is scoped AND it explicitly Dispose()s the variable at the end of the using block.

Retesting the code without a using gives the same exception (the log file is without a using).

The comment by Stuart about always closing data readers made me double check they were all being closed correctly. I found one I had commented out trying to strip the code down:

Code: Select all

                System.Data.SqlClient.SqlDataReader jiwaSalesOrderReader = FirstSalesOrderUsingReferenceReader(order.id.Value.ToString()); //JiwaSalesOrderReader(sql);
//                if (jiwaSalesOrderReader != null && jiwaSalesOrderReader.HasRows)
//                {
//                    // Because a log needs to be written...
//                    logMessage =
//                        string.Format(
//                            Constants.WooCommerce.Messages.Warning.SalesOrderMapExists,
//                            order.id.Value, methodName);
//                    AddWooCommerceWarningLog(
//                        AerpSchema.WooCommerceLog.Directions.WooCommerceToJiwa,
//                        stopwatch.ElapsedMilliseconds,
//                        logMessage);
//
//                    while (jiwaSalesOrderReader.Read())
//                    {
//                        // Because the PK is used...
//                        string invoiceId = jiwaSalesOrderReader[JiwaSchema.SO_Main.InvoiceId] as string;
//                        // ...in the map
//                        WooCommerceIdMap map = new WooCommerceIdMap()
//                        {
//                            JiwaEntity = JiwaSchema.SO_Main.SchemaTableName,
//                            JiwaIdentity = invoiceId,
//                            WooCommerceEntity = Constants.WooCommerce.Entity.SalesOrder,
//                            WooCommerceIdentity = order.id.Value.ToString()
//                        };
//                        AddWooCommerceIdMap(AerpSchema.WooCommerceLog.Directions.WooCommerceToJiwa, map);
//
//                        // Because this is a not-quite-normal situation
//                        stopwatch.Stop();
//                        logMessage =
//                            string.Format(
//                                Constants.WooCommerce.Messages.Information.ExistingSalesOrderMapped,
//                                order.id.Value,
//                                JiwaSchema.SO_Main.SchemaTableName, invoiceId,
//                                methodName);
//                        AddWooCommerceInformationLog(
//                            AerpSchema.WooCommerceLog.Directions.WooCommerceToJiwa,
//                            stopwatch.ElapsedMilliseconds,
//                            logMessage);
//
//                        // Because it needs to be part of the set of maps
//                        idMaps.Add(map);
//
//                        // Because it exists, update it
//                        ImportOrderAsUpdatedJiwaSalesOrder(order, wooCommerceSystemSettings, map, ref processedCount);
//                    }
//               
//               jiwaSalesOrderReader.Close(); // ######################################## Should be outside comment
//                }
//                else
//                {
                    // Moved to here ##############################
                    jiwaSalesOrderReader.Close();
                    stopwatch.Restart();


Moving this outside the commented section has resolved the issue. I will now bring back all of the code and check for Close()s in the appropriate places.

Thanks for all of your assistance with this.

[SBarnes]

Hi Mark

While only very quickly looking at what you are trying to do with your logging so apologies if I have missed some thing because there is a fair bit in what you have sent through if you are trying to log information to a table and get the unique identifier back something that I know works well under Jiwa would be something like the following which takes advantage ORMLite and ServiceStack to achieve a similar outcome in a project I am working on at the moment, which is as follows and might be of some help given it will work in save start or save end events on a sales order and other business objects because the project mentioned does this:


Table

Code: Select all

CREATE TABLE [dbo].[ES_SaveLog](
   [ID] [uniqueidentifier] NOT NULL,
   [ClassName] [varchar](150) NULL,
   [JiwaID] [varchar](36) NULL,
   [Inserted] [datetime] NULL,
   [Updated] [datetime] NULL,
   [SaveGuid] [varchar](36) NULL,
   [WasHook] [bit] NULL,
 CONSTRAINT [PK_ES_SaveLog] PRIMARY KEY CLUSTERED
(
   [ID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO

ALTER TABLE [dbo].[ES_SaveLog] ADD  CONSTRAINT [DF_ES_SaveLog_ID]  DEFAULT (newid()) FOR [ID]
GO

ALTER TABLE [dbo].[ES_SaveLog] ADD  CONSTRAINT [DF_ES_SaveLog_Inserted]  DEFAULT (getdate()) FOR [Inserted]
GO

ALTER TABLE [dbo].[ES_SaveLog] ADD  CONSTRAINT [DF_ES_SaveLog_Updated]  DEFAULT (getdate()) FOR [Updated]
GO



Class Representing the table

Code: Select all

namespace JiwaFinancials.Jiwa.JiwaServiceModel.Tables
{
     [Alias("ES_SaveLog")]
    public partial class ES_SaveLog : IHasId<Guid>
    {
        [Alias("ID")]
        [Required]
        [AutoId]
        public Guid Id { get; set; }
        public string ClassName { get; set; }
        public string SaveGuid { get; set; }
        public string JiwaID { get; set; }
        public DateTime? Inserted { get; set; }
        public DateTime? Updated { get; set; }
        public bool? WasHook  { get; set; }
    }
}


Code to Write to the log

Code: Select all

public static Guid LogChange(JiwaFinancials.Jiwa.JiwaApplication.Manager manager, string ClassName,  string JiwaID, string GUID, bool? WasHook = false)
   {
      var dbFactory = new OrmLiteConnectionFactory(manager.Database.ConnectionString, ServiceStack.OrmLite.SqlServer2012Dialect.Provider);
        using (var db = dbFactory.Open())
        {
         ES_SaveLog log;
         log = db.Where<ES_SaveLog>(new {JiwaID = JiwaID, ClassName = ClassName}).FirstOrDefault();
         if(log == null)
         {
            log = new ES_SaveLog();
            log.ClassName = ClassName;
            log.JiwaID = JiwaID;
            log.SaveGuid = GUID;
            log.Inserted = DateTime.Now;
            log.Updated = DateTime.Now;
            log.WasHook = WasHook;
            db.Insert(log);            
         }
         else
         {
            log.ClassName = ClassName;
            log.JiwaID = JiwaID;
            log.SaveGuid = GUID;
            log.Updated = DateTime.Now;
            log.WasHook = WasHook;
            db.Update(log);               
         }
         Guid itemId = log.Id;
         return itemId;

      }
      
   }


[Mike.Sheen]

Stuart's approach here is good because it uses a separate connection - if you were trying to log into a table and something else throws an exception causing a rollback, you may end up rolling back log entries making it impossible to follow the actual sequence of events in the log.