•  
      request #26737 Have a fork of tlp-fetch using neverthrow and Fault
    Infos
    #26737
    Joris MASSON (jmasson)
    2022-04-29 14:53
    2022-04-28 17:30
    28261
    Details
    Have a fork of tlp-fetch using neverthrow and Fault

    @tuleap/tlp-fetch suffers from several API design issues:

    • it does not encode the URIs it receives, forcing all callers to do it.
    • it is painful to retrieve JSON. All callers of get have to await a second time response.json() to get what they want: the JSON payload. In all but the rarest of cases, callers don't care about the Response, they only want the JSON payload.
    • it is painful to send JSON. All callers of patch, put and post have to set the same "Content-Type" header and have to encode their payload to JSON by themselves.
    • it is painful to handle API errors efficiently. All error-handling code have to import FetchWrapperError (which makes it depend directly on @tuleap/tlp-fetch) and then have to decode the error response JSON and then look for an error.i18n_error_message or an error.message property to display the API's error message.
    • it is painful to handle errors in general. Since @tuleap/tlp-fetch throws Errors (and thus rejects its Promise), callers are expected to wrap it in try/catch blocks. All is well until JavaScript throws a TypeError due to a programmer mistake for example. The TypeError will be caught in the try/catch, same as all errors. Then, depending on the quality of the error-handling code, it will either be silently ignored (which means we don't notice programmer mistakes), it will appear in the error message section shown to the end-user or it will appear in the console, alongside errors that aren't programmer mistakes.
    • it lets callers set fetch init options, but it is so rarely used (never ?) that it does not seem worth it. It complicates the function types a lot for no benefit.

    In the hope of addressing those API design issues, we should write a fork of @tuleap/tlp-fetch. Forking seems the best way because fixing those problems means breaking backwards-compatibility. Since it is no longer part of the big TLP library, we can adopt the new library gradually.

    To address those issues:

    • it should automatically encode the URIs it receives.
    • it should allow to GET a typed JSON payload directly.
    • for "write operations" (PUT, PATCH, POST), it should set automatically the "Content-Type" header to "application/json". It should automatically encode a payload to JSON in the Request body.
    • it should handle the decoding of JSON error and the retrieval of i18n_error_message.
    • it should return a ResultAsync from neverthrow. It allows to easily distinguish "success" and "failure" cases. The return type makes it clear that the operation can fail. Programming mistakes keep throwing TypeErrors, but they should pop up normally in the console (and make the program crash) as try/catch blocks become unnecessary with ResultAsync.
    • it should not allow setting fetch init options.
    Dev tools
    Empty
    Empty
    • [ ] enhancement
    • [x] internal improvement
    Empty
    Stage
    Joris MASSON (jmasson)
    Closed
    2022-04-29
    Attachments
    Empty
    References

    Follow-ups

    User avatar
    Thomas Gerbet (tgerbet)2022-04-29 14:53

    Integrated into Tuleap 13.8.99.7.


    • Status changed from Under review to Closed
    • Connected artifacts
    • Close date set to 2022-04-29