cap-js / docs

CAP Documentation
https://cap.cloud.sap
Apache License 2.0
45 stars 98 forks source link

Node.js - Error Sanitisation & OData V4 Server Messages for Confirmation Popups in Fiori Elements #436

Open sebastianesch opened 1 year ago

sebastianesch commented 1 year ago

Hi,

I'm trying to get OData V4 Server messages to work https://ui5.sap.com/#/topic/fbe1cb5613cf4a40a841750bf813238e to get Confirmation Dialogs in my Fiori Elements applications (see https://ui5.sap.com/#/topic/9a536627a6a94de084b0605eb164d2c8), but it seems as CAP does not allow all properties SAPUI5 supports, but only the ones from the OData standard. Unfortunately SAPUI5 cannot work with "@transition" but expects "transition", which is sanitised by CAP.

It would make sense to state here https://github.com/cap-js/docs/blob/7ec87234640ab542e775a95ef00fbcb7ad7943fa/node.js/events.md?plain=1#L429 that Confirmation Popups in Fiori Elements are not supported by CAP or the additional properties that SAPUI5 supports for errors should not be sanitised.

Kind regards, Sebastian

johannes-vogel commented 11 months ago

Hi @sebastianesch,

I feel that the sanitization is a bit too strict here. As we do not support persistent error messages ootb, I assume we did not have the additional properties in mind.

We'll double check :)

Best regards, Johannes

johannes-vogel commented 10 months ago

HI @sebastianesch,

after additional reading of https://ui5.sap.com/#/topic/fbe1cb5613cf4a40a841750bf813238e section Messages in error responses

Only transition messages are transported in the error response. The messages may be bound or unbound. Error messages are always reported in the error response in JSON format, as described in the OData JSON Format Version 4.0 in Section 19 Error Response, with the following additions: ....

If I read that correctly, the transition property cannot be part of the error response. It can only be part of an error transported in sap-messages header or in the explicitly modeled message property of an entity.

Best, Johannes

sebastianesch commented 7 months ago

Hi @johannes-vogel,

the section of the UI5 documentations above your quote states:

There are three different channels for transporting messages to the client:

  1. The error response for transporting unbound and bound transition messages in the error case.
  2. The sap-messages header for transporting unbound and bound transition messages in the success case.
  3. The message property as part of the entity for transporting bound state and transition messages in the success case.

For the confirmation use case, we report an HTTP Status Code 412, means we are in case 1 and I need the transition property in the response itself. The sap-messages header is used in the success case.

Looking at @sap/cds/libx/_runtime/common/error/constants.js:

  ALLOWED_PROPERTIES_MAP: { code: 1, message: 1, target: 1, details: 1, innererror: 1 },
  ADDITIONAL_MSG_PROPERTIES_MAP: { numericSeverity: 1, longtextUrl: 1, transition: 1 },

all the UI5 properties are there, but there is a distinction between allowed properties and additional properties - which would be useful to propagate to SAPUI5 applications.

After the error is handled in the in the Common Error module, it seems to be processed later again, but I have not figured out when and by which handler.

Kind regards, Sebastian

David-Kunz commented 7 months ago

Notes:

In the OData v4 error response documentation, there's no mention of the transition property. It only allows additional properties via annotations.

I can check with the Fiori Elements/UI5 colleagues.

ThomasChadzelek commented 7 months ago

Hello @sebastianesch !

I cannot comment on CAP, but I am the team architect for UI5's v4.ODataModel. From my perspective, the transition property is not needed for error responses because "Only transition messages are transported in the error response" - which means, we treat all of them as transition messages ;-)

Which issues do you experience with the confirmation use case? Maybe "Strict Handling" on https://openui5.hana.ondemand.com/topic/b54f7895b7594c61a83fa7257fa9d13f helps?

Best regards, Thomas

sebastianesch commented 7 months ago

HI @David-Kunz @ThomasChadzelek,

I have created an example at https://github.com/sebastianesch/sap-cap-examples/tree/master/412-confirmation-dialog. At the moment, if I reply in CAP with an 412 error, the Fiori Elements application treats the response as an error and does not show a confirmation dialog. (See repo for a screenshot)

Looking at the UI5 documentation Confirmation Popup for Actions that Fail with 412 Warnings, it states:

Depending on the settings of the back end, there are two options:

  • If the preference is unknown, it's ignored and the action is executed as usual.
  • If the preference is known, application developers can configure the back end to send a 412 message ("Precondition Failed" message).

At the bottom of the page (for OData V4) is stated:

You must configure 412 messages from the back end as transition messages, not as state messages.

That's why I figured, that the response needs the transition property.

@David-Kunz I'm aware that the OData V4 spec does not describe the transition property, but the Server Messages in the OData V4 Model documentation does.

Kind regards, Sebastian

ThomasChadzelek commented 7 months ago

"if I reply in CAP with an 412 error, the Fiori Elements application treats the response as an error" Are you sure that your response includes "Preference-Applied : handling=strict"? W/o that, the client would not know that the error has been caused by strict handling.

sebastianesch commented 7 months ago

@ThomasChadzelek I don't do that. I haven't seen this in the documentation anywhere. I'll try that.

Adding the header to the response results in the Fiori App staying in the busy state:

image
ThomasChadzelek commented 7 months ago

Hmm, the response now looks as expected, as far as I can tell. Maybe you can share the full response as text here.

Are there any errors on console, apart from that "busy lock"?

sebastianesch commented 7 months ago

The response from the network tab in Chrome is:

--batch_id-1708089594292-130
content-type: application/http
content-transfer-encoding: binary

HTTP/1.1 412 Precondition Failed
odata-version: 4.0
preference-applied: handling=strict
content-type: application/json;odata.metadata=minimal;IEEE754Compatible=true

{"error":{"code":"412","message":"Precondition Failed: Confirm project closure","@Common.numericSeverity":4}}
--batch_id-1708089594292-130--

When I delete the console before executing the action, I don't get any output in the Chrome Console immediately. After 30 seconds I get:

Log-dbg.js:499 2024-02-16 14:21:35.810500 busy lock for id-1708089648579-17/busy with value 1 timed out after 30 seconds! -  
g @ Log-dbg.js:499
t.error @ Log-dbg.js:249
(anonymous) @ BusyLocker.ts:57
setTimeout (async)
s @ BusyLocker.ts:56
_updateLock @ BusyLocker.ts:94
lock @ BusyLocker.ts:80
e @ TransactionHelper.ts:74
onSubmitted @ TransactionHelper.ts:912
S @ facade.ts:1850
(anonymous) @ facade.ts:1894
X @ facade.ts:1721
(anonymous) @ facade.ts:406
H @ facade.ts:220
_ @ facade.ts:110
e @ TransactionHelper.ts:902
t @ EditFlow.ts:1979
await in t (async)
(anonymous) @ ControllerExtension-dbg.js:104
t @ TableAPI.ts:1489
n.<computed> @ ClassSupport.ts:300
y @ ExpressionParser-dbg.js:412
(anonymous) @ ExpressionParser-dbg.js:735
l @ ExpressionParser-dbg.js:915
l.getExternalValue @ CompositeBinding-dbg.js:309
u @ EventHandlerResolver-dbg.js:349
(anonymous) @ EventHandlerResolver-dbg.js:173
i.fireEvent @ EventProvider-dbg.js:241
c.fireEvent @ Element-dbg.js:659
(anonymous) @ ManagedObjectMetadata-dbg.js:826
S.ontap @ Button-dbg.js:599
c._handleEvent @ Element-dbg.js:345
N._handleEvent @ UIArea-dbg.js:1050
dispatch @ jquery-dbg.js:5430
c @ jquery-mobile-custom-dbg.js:1907
d @ jquery-mobile-custom-dbg.js:2030
dispatch @ jquery-dbg.js:5430
y.handle @ jquery-dbg.js:5234
trigger @ jquery-dbg.js:8823
(anonymous) @ jquery-dbg.js:8901
each @ jquery-dbg.js:385
each @ jquery-dbg.js:207
trigger @ jquery-dbg.js:8900
j @ jquery-mobile-custom-dbg.js:1455
L @ jquery-mobile-custom-dbg.js:1465
dispatch @ jquery-dbg.js:5430
y.handle @ jquery-dbg.js:5234
ThomasChadzelek commented 7 months ago

OK, our docs are for sure never precise enough ;-) The response is insufficient. I cannot exactly explain how it leads to the issue you observe, but I can explain what is missing.

When an action fails due to "strict handling", it means that the action would have succeeded without strict handling, but that warnings occured which have been escalated, so to say, to become errors instead (that's the core of strict handling). This means that the error response needs to contain an error object (check) with all these warnings as details - this is missing. Here is an example of such details from our integration test (lines 47920ff.):

          details : [{
              "@Common.numericSeverity" : 3,
              code : "CODE1",
              "@SAP__core.ContentID" : "0.0",
              message : "Note is empty",
              target : "SalesOrder/Note"
          }, {
              "@Common.numericSeverity" : 3,
              code : "CODE1",
              "@SAP__core.ContentID" : "0.0",
              message : "Note is empty",
              target : "SalesOrder/Note"
          }, {
              "@Common.numericSeverity" : 2,
              code : "CODE2",
              "@SAP__core.ContentID" : "0.0",
              message : "Some unbound info"
          }]
333sachin commented 3 months ago

not sure, if this is related, but I get this error when adding composition child entity to a main entity image