Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversion of a thrown exceptions with unknown error codes or 404 not happening #485

Closed
KLarpen opened this issue Dec 20, 2023 · 1 comment · Fixed by #486
Closed

Conversion of a thrown exceptions with unknown error codes or 404 not happening #485

KLarpen opened this issue Dec 20, 2023 · 1 comment · Fixed by #486
Labels
bug Something isn't working

Comments

@KLarpen
Copy link
Contributor

KLarpen commented Dec 20, 2023

Describe the bug

Expected conversion of a thrown exceptions from API endpoints doesn't happening. Message with unknown error code still returns to the client as is without been converted into 500 Internal Server Error. Message with 404 error code also returns to the client as is without been replaced by "Not Found".

To Reproduce

  1. Start with Example project
  2. Run server
  3. Open http://127.0.0.1:8000 in the Chrome with DevTools (console & network tabs in it)
  4. Click on api WebSocket request and there on Messages tab. Check that auth/signin request was successful with the status logged.
  5. Type in DevTools console await api.example.customException(); and run
  6. The result {message: "Custom ecxeption", code: 12345} despite expectation that unknown error code will generate: "Internal Server Error" with "code":500. Server log:
09:12:22  W2   error   127.0.0.1	GET	/api	12345	12345	Error: Custom ecxeption
  /application/api/example/customException.js:2:9
  Procedure.invoke (/node_modules/impress/lib/procedure.js:76:9)
  Server.rpc (/node_modules/metacom/lib/server.js:267:27)
  process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  1. Put the line throw new Error('Custom exception', 12345); into rounter method at application/api/hook.1.js
  2. Open second browser tab with DevTools and query there http://localhost:8001/api/hook/something
  3. You will receive 500 Internal Server Error in the error message and as a HTTP status code header. Server log:
09:23:22  W2   error   127.0.0.1	GET	/api/hook/something	500	500	Error: Example exception
  router (/application/api/hook.1.js:6:11)
  Procedure.invoke (/node_modules/impress/lib/procedure.js:76:9)
  Server.hook (/node_modules/metacom/lib/server.js:346:33)
  Server.request (/node_modules/metacom/lib/server.js:336:20)
  Server.<anonymous> (/node_modules/metacom/lib/server.js:196:12)
  process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  1. Change error code in the file application/api/example/customException.js to 404. Save and check that update has been reloaded by server.
  2. Make the same change in application/api/hook.1.js.
  3. In the console of the 1st browser tab call again await api.example.customException();
  4. The result {message: "Custom ecxeption", code: 404} despite expectation of automatic replacement of the message with "Not found". Server log:
09:36:28  W2   error   127.0.0.1	GET	/api	404	404	Error: Custom ecxeption
  /application/api/example/customException.js:2:9
  Procedure.invoke (/node_modules/impress/lib/procedure.js:76:9)
  Server.rpc (/node_modules/metacom/lib/server.js:267:27)
  process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  1. In the 2nd browser tab query again http://localhost:8001/api/hook/something
  2. You will receive 500 Internal Server Error in the error message and as a HTTP status code header. Exactly the same behavior as it was with custom code webhook's exception. Server log:
09:40:41  W2   error   127.0.0.1	GET	/api/hook/something	500	500	Error: Custom exception
  router (/application/api/hook.1.js:6:11)
  Procedure.invoke (/node_modules/impress/lib/procedure.js:76:9)
  Server.hook (/node_modules/metacom/lib/server.js:346:33)
  Server.request (/node_modules/metacom/lib/server.js:336:20)
  Server.<anonymous> (/node_modules/metacom/lib/server.js:196:12)
  process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Expected behavior

The documentation section Error-handling guidelines contains the sentences

How to override error codes: throw new Error('Method is not implemented', 404);
This will take error message from code: {"callback":1,"error":{"message":"Not found","code":404}}

If you specify unknown code like this: throw new Error('Method is not implemented', 12345); this will generate: "Internal Server Error" with "code":500.

Desktop:

  • OS: macOS 14.1.2 Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64
  • Node.js version 20.9.0
  • Impress: 3.0.13
  • Metacom 3.1.2

##Additional context

Necessity to report a bug originates from metarhia/Docs#21 that had been discussed at the community call 133. Decision to make issue in Metacom repository based on discovery that thrown exceptions is processed by Server.rpc OR Server.hook and in both situations going to Transport.error.

@KLarpen
Copy link
Contributor Author

KLarpen commented Dec 20, 2023

I had investigated that thrown error code conversion for normal API endpoints happens in Server.rpc method

metacom/lib/server.js

Lines 266 to 273 in 79b00cb

try {
result = await proc.invoke(context, args);
} catch (error) {
let code = error.code === 'ETIMEOUT' ? 408 : 500;
if (typeof error.code === 'number') code = error.code;
error.httpCode = code;
return void client.error(code, { id, error });
} finally {

Webhooks always return 500 Internal Server Error for all thrown exceptions as they processed in specific method Server.hook

metacom/lib/server.js

Lines 344 to 350 in 79b00cb

try {
const par = { verb, method, args, headers };
const result = await proc.invoke(context, par);
client.send(result);
} catch (error) {
client.error(500, { id, error });
}

Possible conversion of the error message to be responded to client happens in Transport.error based on discovered httpCode

metacom/lib/transport.js

Lines 100 to 103 in 79b00cb

if (!httpCode) httpCode = (error && error.httpCode) || code;
const status = http.STATUS_CODES[httpCode];
const pass = httpCode < 500 || httpCode > 599;
const message = pass && error ? error.message : status || 'Unknown error';

Note that method used to send both: domain errors and thrown exceptions.

KLarpen added a commit to KLarpen/metacom that referenced this issue Dec 20, 2023
Notice: custom exceptions with error code less then 500 still be passed as is (neither code not message won't be converted). The basis of the decision — potential necessity to imitate client error responses (400-499).

Closes: metarhia#485
tshemsedinov pushed a commit that referenced this issue Feb 3, 2024
Fix conversion of a thrown exceptions with unknown error codes

Notice: custom exceptions with error code less then 500 still be passed as is (neither code not message won't be converted). The basis of the decision — potential necessity to imitate client error responses (400-499).

Closes: #485
PR-URL: #486
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant