5

My client wants an insert trigger on his Order table (from Sage) to create a Freshdesk ticket using the API.

As part of my development, I built a stored procedure that does the job fine when provided with an order number. However, transplanting the same code into a trigger returns without error, but nothing appears in the Freshdesk system, when the same code in a stored procedure works.

I expect comments about why an API call in a trigger might be a bad idea, but the Freshdesk call is very quick (<1 second from the stored procedure).

What I'd like to know is -- Is this is architecturally forbidden by SQL Server for some reason? If it's allowed, where might I look for the error that's being thrown.

Edit2: OK, here's the whole trigger .. pervious version just had OA calls.

ALTER TRIGGER [dbo].[CreateFreshdeskTicketFromOrder] 
   ON  [dbo].[OEORDH] 
   AFTER INSERT
AS 
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

  -- Get the original order number, and use that in the main lookup query
  DECLARE @ORDNUM VARCHAR(22)
  SELECT @ORDNUM = ORDNUMBER FROM inserted

  -- Variables for fields going to the API
  DECLARE @EMAIL VARCHAR(60), @SHPCONTACT VARCHAR(60), @ORDNUMBER VARCHAR(22)
  DECLARE @LOCATION VARCHAR(6), @EXPDATE INT
  DECLARE @SHPPHONEC VARCHAR(30), @SHPNAME VARCHAR(60), @DESCR VARCHAR(60)
  DECLARE @CODEEMPL VARCHAR(15)

  -- Collect field values that were just inserted
  SELECT
    @EMAIL = rtrim(OEORDH1.SHPEMAILC), @SHPCONTACT = rtrim(SHPCONTACT),
    @ORDNUMBER = rtrim(ORDNUMBER), @LOCATION = LOCATION, @EXPDATE = EXPDATE,
    @SHPPHONEC = rtrim(OEORDH1.SHPPHONEC), @SHPNAME = SHPNAME,
    @DESCR = rtrim([DESC]), @CODEEMPL = rtrim(ARSAP.CODEEMPL)
  -- FROM inserted
  FROM dbo.OEORDH
  JOIN dbo.OEORDH1 on dbo.OEORDH.ORDUNIQ   = dbo.OEORDH1.ORDUNIQ
  JOIN dbo.ARSAP   on dbo.OEORDH.SALESPER1 = dbo.ARSAP.CODESLSP
  WHERE ORDNUMBER = @ORDNUM

  -- Variables from database to the API call
  DECLARE @EXPDATE_OUT VARCHAR(10)
  SET @EXPDATE_OUT =
    substring ( cast ( @EXPDATE as varchar(8) ), 1, 4 ) + '-' +
    substring ( cast ( @EXPDATE as varchar(8) ), 5, 2 ) + '-' +
    substring ( cast ( @EXPDATE as varchar(8) ), 7, 2 );

  DECLARE @STATUS_OUT VARCHAR(2)
  IF @LOCATION = '1A'
    SET @STATUS_OUT = '23';
  ELSE
    IF @LOCATION = '1'
      SET @STATUS_OUT = '40';
    ELSE
      SET @STATUS_OUT = '2';

  -- Variables for building the API call
  DECLARE @Object INT
  DECLARE @Url VARCHAR(80)

  DECLARE @Body1 VARCHAR(1000) =
  '{ ' +
    '"email": "'+ @EMAIL +'", ' +
    '"custom_fields": { "order_number": "'+ @ORDNUMBER +'", "scheduled_date": "'+ @EXPDATE_OUT + '", ' +
    '"delivered_to": "'+ @SHPCONTACT + '", ' + '"consignee_phone_number": "'+ @SHPPHONEC +'" }, ' +
    '"status": '+ @STATUS_OUT + ', ' +
    '"priority": 1, "subject": "'+ rtrim(@ORDNUMBER) + ' - ' + rtrim(@SHPNAME) + ' (' + @DESCR + ')", ' +
    '"responder_id": ' + @CODEEMPL +
  ' }'

  DECLARE @ResponseText VARCHAR(1000), @return_status INT

  SET @Url = 'https://client.freshdesk.com/api/v2/tickets';

  -- Do REST call to API / All return statuses commented out except for last
  Exec @return_status = sp_OACreate 'MSXML2.ServerXMLHTTP', @Object OUT;
  -- Select 'Create return', @return_status

  Exec @return_status = sp_OAMethod @Object, 'Open', NULL, 'POST', @Url, false
  -- Select 'Open return', @return_status

  Exec @return_status = sp_OAMethod @Object, 'setRequestHeader', NULL,
    'Content-Type', 'application/json'
  -- Select 'Set Request Header1 return', @return_status
  Exec @return_status = sp_OAMethod @Object, 'setRequestHeader', NULL,
    'Authorization', 'Basic ABC123=='
  -- Select 'Set Request Header2 return', @return_status

  Exec @return_status = sp_OAMethod @Object, 'Send', NULL, @Body1
  -- Select 'Send1 return', @return_status

  Exec sp_OAMethod @Object, 'ResponseText', @ResponseText OUT
  -- Select 'Response', @ResponseText

  Exec sp_OADestroy @Object

  -- Add the conversation to the TriggerLog

  IF @ResponseText IS NULL
    SET @ResponseText = '(Null)';

  INSERT INTO dbo.TriggerLog (tl_source, tl_input, tl_output) VALUES
    ( 'FreshdeskInsertTrigger', @Body1, @ResponseText )
END

That's the trigger code.

A stored procedure that has the same code (but takes an order number as a parameter) works correctly and does the API call and does the logging. Commenting out the logging at the end of the trigger made the error from Sage go away, but the API call still didn't arrive.

Vladimir Baranov
  • 31,799
  • 5
  • 53
  • 90
talexb
  • 135
  • 1
  • 8
  • No, it is not forbidden. Typically, the problem is in the code written for the trigger. Search the posts for the **many** trigger related questions which almost always involve a discussion about poor coding practices, invalid assumptions, and better solutions. Either that or post the trigger code. – SMor Feb 25 '18 at 04:23
  • Here's the trigger code: – talexb Feb 25 '18 at 05:06
  • No - that is not your trigger code. At best, is it only part of your trigger code. Post **ALL** of it - and then describe what it does. Since you claim it is working in a stored procedure, then perhaps your easiest (which does not mean "best" by any means) approach is to call your procedure within the trigger. And you certainly do need to add error handling. – SMor Feb 25 '18 at 13:52
  • 2
    "the Freshdesk call is very quick" - that may be so - when everything is working correctly. Now imagine that there's a DNS issue that causes a lookup failure (after 30 seconds). Or there's a problem on the network path. Or freshdesk are experiencing an outage of some kind. Those are the sort of things you should consider before deciding to risk the original transaction that's trying to insert an order. And why it's almost always better to introduce some form of queueing (to e.g. service broker) so that network issues don't affect the original transaction. – Damien_The_Unbeliever Feb 27 '18 at 07:29
  • I agree that in the long-term, this solution may break. However, I'm trying to understand why it's not working in the present, when there are no DNS problems. The code appears to work well when it's run manually from a stored procedure. It also appears to fail silently when it's run automatically from within a trigger, and I'm trying to understand why. – talexb Feb 27 '18 at 14:08

1 Answers1

3

What happens if you simply call your working stored procedure from the trigger (via EXEC) instead of including the procedure's code in the trigger?


One thing to look closely at is this place in the code:

  -- Collect field values that were just inserted
  SELECT
    @EMAIL = rtrim(OEORDH1.SHPEMAILC), @SHPCONTACT = rtrim(SHPCONTACT),
    @ORDNUMBER = rtrim(ORDNUMBER), @LOCATION = LOCATION, @EXPDATE = EXPDATE,
    @SHPPHONEC = rtrim(OEORDH1.SHPPHONEC), @SHPNAME = SHPNAME,
    @DESCR = rtrim([DESC]), @CODEEMPL = rtrim(ARSAP.CODEEMPL)
  -- FROM inserted
  FROM dbo.OEORDH
  JOIN dbo.OEORDH1 on dbo.OEORDH.ORDUNIQ   = dbo.OEORDH1.ORDUNIQ
  JOIN dbo.ARSAP   on dbo.OEORDH.SALESPER1 = dbo.ARSAP.CODESLSP
  WHERE ORDNUMBER = @ORDNUM

You commented out FROM inserted and try to read values from the table directly.

When the trigger code runs the transaction is not committed yet, so most likely you should read the values from the inserted table. It is likely that this SELECT doesn't find a row with the given @ORDNUM and variables remain NULL.


Side note. Triggers in SQL Server are fired once per the statement, not once per row. Your trigger should work correctly even if inserted table has several rows. Right now your trigger would pick only one ORDNUMBER even if several rows were inserted into the table. Most likely this is not what you want.


How to debug the trigger?

One simple and very straight-forward way is to create a table for logging and add plenty of INSERT statements that would log values of all variables into that table. Then you can read through the logs and see what was going on.

Vladimir Baranov
  • 31,799
  • 5
  • 53
  • 90
  • Thank you. What isn't explained (or, more exactly, what I didn't understand) is that the universe of a trigger is one where things Don't Officially Exist Yet. The multi-table join I'm doing involves a second table (OEORDH1) whose record may also Not Yet Officially Exist, causing the JOIN to fail, thus the trigger returns an error. – talexb Feb 28 '18 at 05:23
  • @talexb, if you need to get data from other tables in addition to the table on which the trigger is defined, you need to be careful. I don't have much experience with such triggers, but I would try to start a transaction explicitly, add the needed data to the "other" table first (`OEORDH1`), then insert in the main table and the trigger should fire within the same transaction. Commit transaction explicitly only after inserting into the main table. You'll need to check, but I'd expect the trigger to "see" the new data in "other" table if it was inserted in the same transaction as the main table – Vladimir Baranov Feb 28 '18 at 05:49
  • 2
    Triggers run within the context of the transaction that covers the original statement. As such, your concern about not being able "It is likely that this SELECT doesn't find a row with the given @ORDNUM..." isn't valid. In just the same way that within a single transaction you can always read back rows that *you* have inserted. – Damien_The_Unbeliever Feb 28 '18 at 08:59
  • @Damien_The_Unbeliever, you are right. The rows that are visible in the `inserted` table are also visible if you reference the original table directly. It turns out that in this specific case there is another table involved that is also changing (and it is not clear how exactly it is changing) and that may complicate everything. – Vladimir Baranov Feb 28 '18 at 10:06