data-api-builder icon indicating copy to clipboard operation
data-api-builder copied to clipboard

[Bug]: Optional stored procedure parameters generate the error "Missing required procedure parameters"

Open yorek opened this issue 2 years ago • 19 comments

What happened?

I have a stored procedure like the following:

create procedure dbo.stp_Dummy
@mandatoryParam int,
@optionalParam int = 10
as
select 
    @mandatoryParam as mandatoryParam,
    @optionalParam as optionalParam
go  

for which the @optionalParam is, as the name implies, optional.

If I configure DAB to use that stored procedure using the following configuration, omitting the optionalParam (since it is optional):

"Dummy": {
    "source": {
      "type": "stored-procedure",
      "object": "dbo.stp_Dummy",
      "parameters": {
        "mandatoryParam": 10
      }
    },
    "graphql": {
      "operation": "query"
    },
    "permissions": [
      {
        "role": "anonymous",
        "actions": [
          "execute"
        ]
      }
    ]
  }

I get the following error during the startup:

 Azure.DataApiBuilder.Service.Exceptions.DataApiBuilderException: Invalid request. Missing required procedure parameters: optionalParam for entity: Dummy

Version

Microsoft.DataApiBuilder 0.8.52+c69925060e1942d28515b9c4b89ea24832da0c7c

What database are you using?

Azure SQL

What hosting model are you using?

Local (including CLI)

Which API approach are you accessing DAB through?

REST, GraphQL

Relevant log output

Information: Microsoft.DataApiBuilder 0.8.52+c69925060e1942d28515b9c4b89ea24832da0c7c
Information: Config not provided. Trying to get default config based on DAB_ENVIRONMENT...
Information: Environment variable DAB_ENVIRONMENT is (null)
Loading config file from dab-config.json.
Information: Loaded config file: dab-config.json
Information: Setting default minimum LogLevel: Debug for Development mode.
Starting the runtime engine...
Loading config file from dab-config.json.
info: Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager[63]
      User profile is available. Using 'C:\Users\damauri\AppData\Local\ASP.NET\DataProtection-Keys' as key repository and Windows DPAPI to encrypt keys at rest.
info: Azure.DataApiBuilder.Core.Services.ISqlMetadataProvider[0]
      Dummy path: /api/Dummy
dbug: Azure.DataApiBuilder.Core.Resolvers.IQueryExecutor[0]
      Executing query:
SELECT name as result_field_name, TYPE_NAME(system_type_id) as result_type, is_nullable FROM sys.dm_exec_describe_first_result_set_for_object (OBJECT_ID('dbo.stp_Dummy'), 0) WHERE is_hidden is not NULL AND is_hidden = 0
dbug: Azure.DataApiBuilder.Core.Services.ISqlMetadataProvider[0]
      Logging primary key information for entity: Dummy.
info: Azure.DataApiBuilder.Core.Configurations.RuntimeConfigValidator[0]
      Validating Relationship Section in Config...
fail: Azure.DataApiBuilder.Service.Startup[0]
      Unable to complete runtime initialization. Refer to exception for error details.
      Azure.DataApiBuilder.Service.Exceptions.DataApiBuilderException: Invalid request. Missing required procedure parameters: optionalParam for entity: Dummy
         at Azure.DataApiBuilder.Core.Configurations.RuntimeConfigValidator.ValidateStoredProceduresInConfig(RuntimeConfig runtimeConfig, ISqlMetadataProvider sqlMetadataProvider)
         at Azure.DataApiBuilder.Service.Startup.PerformOnConfigChangeAsync(IApplicationBuilder app)
fail: Azure.DataApiBuilder.Service.Startup[0]
      Exiting the runtime engine...
crit: Microsoft.AspNetCore.Hosting.Diagnostics[6]
      Application startup exception
      System.ApplicationException: Could not initialize the engine with the runtime config file: dab-config.json
         at Azure.DataApiBuilder.Service.Startup.Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeConfigProvider runtimeConfigProvider)
         at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
         at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
         at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
         at Microsoft.AspNetCore.Hosting.ConfigureBuilder.<>c__DisplayClass4_0.<Build>b__0(IApplicationBuilder builder)
         at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.<>c__DisplayClass15_0.<UseStartup>b__1(IApplicationBuilder app)  
         at Microsoft.AspNetCore.Mvc.Filters.MiddlewareFilterBuilderStartupFilter.<>c__DisplayClass0_0.<Configure>g__MiddlewareFilterBuilder|0(IApplicationBuilder builder)
         at Microsoft.AspNetCore.HostFilteringStartupFilter.<>c__DisplayClass0_0.<Configure>b__0(IApplicationBuilder app)       
         at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
Unable to launch the runtime due to: System.ApplicationException: Could not initialize the engine with the runtime config file: dab-config.json
   at Azure.DataApiBuilder.Service.Startup.Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeConfigProvider runtimeConfigProvider)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
   at Microsoft.AspNetCore.Hosting.ConfigureBuilder.<>c__DisplayClass4_0.<Build>b__0(IApplicationBuilder builder)
   at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.<>c__DisplayClass15_0.<UseStartup>b__1(IApplicationBuilder app)        
   at Microsoft.AspNetCore.Mvc.Filters.MiddlewareFilterBuilderStartupFilter.<>c__DisplayClass0_0.<Configure>g__MiddlewareFilterBuilder|0(IApplicationBuilder builder)
   at Microsoft.AspNetCore.HostFilteringStartupFilter.<>c__DisplayClass0_0.<Configure>b__0(IApplicationBuilder app)
   at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
   at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.Run(IHost host)
   at Azure.DataApiBuilder.Service.Program.StartEngine(String[] args)
Error: Failed to start the engine.

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

yorek avatar Sep 26 '23 18:09 yorek

this is related to this: https://github.com/Azure/data-api-builder/issues/989. There is a validation error that is done on the runtimeconfig where we check that number of params on stored proc should match number in runtimeconfig, which is incorrect. We had reported this during our sync ups. I have the fix for this and will check it in soon.

rohkhann avatar Sep 26 '23 20:09 rohkhann

@rohkhann and @abhishekkumams can you check if #1847 does indeed resolve this issue too? if so, please link the pr item and close this.

seantleonard avatar Feb 08 '24 19:02 seantleonard

@rohkhann to confirm whether this is resolved.

seantleonard avatar Mar 06 '24 23:03 seantleonard

We're hitting this issue

simonsabin avatar Mar 22 '24 12:03 simonsabin

@simonsabin, which version of DAB are you using?

seantleonard avatar Mar 26 '24 16:03 seantleonard

Whatever you get with the service. We changed a proc and added optional parameters and the service died.

Simon Sabin


From: Sean Leonard @.> Sent: Tuesday, March 26, 2024 4:50:57 PM To: Azure/data-api-builder @.> Cc: Simon Sabin @.>; Mention @.> Subject: Re: [Azure/data-api-builder] [Bug]: Optional stored procedure parameters generate the error "Missing required procedure parameters" (Issue #1748)

@simonsabinhttps://github.com/simonsabin, which version of DAB are you using?

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/data-api-builder/issues/1748#issuecomment-2020971434, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJHM22LHX67QFWWZOQ6LLDY2GRPDAVCNFSM6AAAAAA5IE2UZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRQHE3TCNBTGQ. You are receiving this because you were mentioned.Message ID: @.***>

simonsabin avatar Mar 26 '24 17:03 simonsabin

Now, if you have parameters in your stored proc, you dont need to duplicate those parameters on the config file. Thats what this fix handles: https://github.com/Azure/data-api-builder/pull/1847. However, we still dont have the capability to detect an optional parameter if it is not specified in the call to the stored proc.

rohkhann avatar Mar 28 '24 19:03 rohkhann

Thats a massive problem as there is no way to deploy the change without breaking the system. Which ever is deployed first, DAB or DB, the result is DAB fails to run until both are deployed

simonsabin avatar Mar 28 '24 19:03 simonsabin

I have hit this issue today. What is a good work-around for optional parameters?

BenMenegazzo avatar Jun 21 '24 06:06 BenMenegazzo

Hitting a similar issue as well, with a "Missing required procedure parameters" during a call to a stored proc with an optional parameter (not declared in the dab config file) without passing this parameter for this call. Please see #2245

Benjiiim avatar Jul 07 '24 21:07 Benjiiim

I believe this should be removed https://github.com/Azure/data-api-builder/blob/689c3e7affe5b94d3ea5d7452c49235068e42652/src/Core/Resolvers/Sql%20Query%20Structures/SqlExecuteQueryStructure.cs#L71 or some configuration to remove it. Sure there is value at design time if the configuration hasn't been setup correctly to provide a nicer error, but you will still get an error if you don't specify all the required parameters.

simonsabin avatar Jul 08 '24 14:07 simonsabin

On my end, this is the Exception I'm hitting when trying to call a stored procedure without providing a value for a parameter for which the stored procedure has a default value:

https://github.com/Azure/data-api-builder/blob/689c3e7affe5b94d3ea5d7452c49235068e42652/src/Core/Services/RequestValidator.cs#L173-L177

Benjiiim avatar Jul 08 '24 15:07 Benjiiim

That as well then. I'd do a PR but think this needs a discussion on whether its by default or configurable.

simonsabin avatar Jul 08 '24 15:07 simonsabin

@seantleonard, @rohkhann, @abhishekkumams > may I ask you your thoughts on this one?

Benjiiim avatar Jul 24 '24 20:07 Benjiiim

@benjiiim, agreed with your and @simonsabin's finding. Working on a fix that incorporate both of your experiences encountering errors.

There isn't a straightforward method to determine whether a stored procedure has a default valued parameter and what that value is without parsing the object definition which is error prone.

  • https://stackoverflow.com/questions/63581531/sql-statement-to-retrieve-procedure-optional-parameters-list

Instead, the change I'm making essentially defers error handling to the database. In this case, error 201:

"201", // Procedure or function '%.*ls' expects parameter '%.*ls', which was not supplied.

SELECT message_id AS Error,
    severity AS Severity,
    [Event Logged] = CASE is_event_logged
        WHEN 0 THEN 'No' ELSE 'Yes'
        END,
    [text] AS [Description]
FROM sys.messages
WHERE language_id = 1033 
and message_id = 201

Consequently, DAB will not fail startup or terminate a request when DAB doesn't detect an optional parameter in the request body when DAB runtime config doesn't define a default value for that optional parameter.

What do you think?

seantleonard avatar Jul 25 '24 19:07 seantleonard

Sounds great.

I think there is a case that needs to be dealt with and that’s a parameter that’s defined in the config that’s not supplied and no default.

Ps would personally love to know how to code this change and the associated tests. Would you be interested in working on it with together ?

Simon Sabin


From: Sean Leonard @.> Sent: Thursday, July 25, 2024 8:01:59 PM To: Azure/data-api-builder @.> Cc: Simon Sabin @.>; Mention @.> Subject: Re: [Azure/data-api-builder] [Bug]: Optional stored procedure parameters generate the error "Missing required procedure parameters" (Issue #1748)

@Benjiiimhttps://github.com/Benjiiim, agreed with your and @simonsabinhttps://github.com/simonsabin's finding. Working on a fix that incorporate both of your experiences encountering errors.

There isn't a straightforward method to determine whether a stored procedure has a default valued parameter and what that value is without parsing the object definition which is error prone.

  • https://stackoverflow.com/questions/63581531/sql-statement-to-retrieve-procedure-optional-parameters-list

Instead, the change I'm making essentially defers error handling to the database. In this case, error 201:

"201", // Procedure or function '%.*ls' expects parameter '%.*ls', which was not supplied.

SELECT message_id AS Error, severity AS Severity, [Event Logged] = CASE is_event_logged WHEN 0 THEN 'No' ELSE 'Yes' END, [text] AS [Description] FROM sys.messages WHERE language_id = 1033 and message_id = 201

Consequently, DAB will not fail startup or terminate a request when DAB doesn't detect an optional parameter in the request body when DAB runtime config doesn't define a default value for that optional parameter.

What do you think?

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/data-api-builder/issues/1748#issuecomment-2251207527, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJHM24ILU5CUEKRCEF57S3ZOFDSPAVCNFSM6AAAAAA5IE2UZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJRGIYDONJSG4. You are receiving this because you were mentioned.Message ID: @.***>

simonsabin avatar Jul 25 '24 22:07 simonsabin

Hi Simon,

I have a working branch here: https://github.com/Azure/data-api-builder/tree/dev/sean/sp_opt_1748

I'd be happy to help you work on your suggestion:

I think there is a case that needs to be dealt with and that’s a parameter that’s defined in the config that’s not supplied and no default.

To clarify, when you say parameter defined in the config file, do you mean this? https://learn.microsoft.com/azure/data-api-builder/reference-configuration#parameters

{
  "entities": {
    "<entity-name>": {
      ...
      "type": "stored-procedure",
      "parameters": {
        "<parameter-name-1>" : "<default-value>",
        "<parameter-name-2>" : "<default-value>",
        "<parameter-name-3>" : "<default-value>"
      }
    }
  }
}

a value is required when defining params in the config. Perhaps you encountered a bug where an invalid, empty, or null value is provided for the parameter and DAB isn't handling properly?

seantleonard avatar Jul 25 '24 22:07 seantleonard

It’s just about what gets validated where and what’s controllable and how. Thinking about it, what is validating data types, default values. I as was pointed out there were a number of places in the code base this touched and on my investigation some didn’t have test cases thus the desire to understand how we put tests in place for this stuffs

simonsabin avatar Jul 25 '24 22:07 simonsabin

I see. Do you have a few test cases in mind? That way I can better guide you were those can be added.

seantleonard avatar Jul 25 '24 23:07 seantleonard