jdalrymple / gitbeaker

🦊🧪 A comprehensive and typed Gitlab SDK for Node.js, Browsers, Deno and CLI
Other
1.55k stars 294 forks source link

Bad Request on Merge Request Note creation (POST) #3595

Open pvignau opened 4 months ago

pvignau commented 4 months ago

Description

Hey there, I don't know why but I can't create a note in a merge request. I know that the package suports Gitlab to its 16.5 version and I'm under 17 but the API does not seem to be different. https://docs.gitlab.com/ee/api/notes.html#create-new-merge-request-note https://archives.docs.gitlab.com/16.5/ee/api/notes.html#create-new-merge-request-note

I don't know if this is a bug of the package or the gitlab server. Am I the only one to experience this ? Thanks for your help !

Steps to reproduce

const targetNote = await findNote();
if (targetNote) {
 await api.MergeRequestNotes.edit(PROJECT_ID, CI_MERGE_REQUEST_IID, targetNote.id, { body: `${comment}` });
} else {
 await api.MergeRequestNotes.create(PROJECT_ID, CI_MERGE_REQUEST_IID, { body: `${comment}` })
}

Expected behaviour

If targetNote is undefined a new one is created.

Actual behaviour

I have a Bad Request returned from the server

{
  description: undefined,
  request: Request {
    [Symbol(realm)]: { settingsObject: [Object] },
    [Symbol(state)]: {
      method: 'POST',
      localURLsOnly: false,
      unsafeRequest: false,
      body: [Object],
      client: [Object],
      reservedClient: null,
      replacesClientId: '',
      window: 'client',
      keepalive: false,
      serviceWorkers: 'all',
      initiator: '',
      destination: '',
      priority: null,
      origin: 'client',
      policyContainer: 'client',
      referrer: 'client',
      referrerPolicy: '',
      mode: 'cors',
      useCORSPreflightFlag: false,
      credentials: 'same-origin',
      useCredentials: false,
      cache: 'default',
      redirect: 'follow',
      integrity: '',
      cryptoGraphicsNonceMetadata: '',
      parserMetadata: '',
      reloadNavigation: false,
      historyNavigation: false,
      userActivation: false,
      taintedOrigin: false,
      redirectCount: 0,
      responseTainting: 'basic',
      preventNoCacheCacheControlHeaderModification: false,
      done: false,
      timingAllowFailed: false,
      headersList: [HeadersList],
      urlList: [Array],
      url: [URL]
    },
    [Symbol(signal)]: AbortSignal { aborted: false },
    [Symbol(abortController)]: AbortController { signal: AbortSignal { aborted: false } },
    [Symbol(headers)]: HeadersList {
      cookies: null,
      [Symbol(headers map)]: [Map],
      [Symbol(headers map sorted)]: null
    }
  },
  response: Response {
    [Symbol(realm)]: null,
    [Symbol(state)]: {
      aborted: false,
      rangeRequested: false,
      timingAllowPassed: true,
      requestIncludesCredentials: true,
      type: 'default',
      status: 400,
      timingInfo: [Object],
      cacheState: '',
      statusText: 'Bad Request',
      headersList: [HeadersList],
      urlList: [Array],
      body: [Object]
    },
    [Symbol(headers)]: HeadersList {
      cookies: null,
      [Symbol(headers map)]: [Map],
      [Symbol(headers map sorted)]: null
    }
  }
}

Checklist

pvignau commented 4 months ago

AFAIK, the problem is not on the package side but on gitlab. If the POST request is made with query params, it works ...

axios.post(
  `https://FQDN/api/v4/projects/${PROJECT_ID}/merge_requests/${CI_MERGE_REQUEST_IID}/notes?body=${encodeURIComponent(comment)}`,
  {},
  config
)

is ok ..

Question answered 🤷‍♂️, closing the issue, sorry for the spam

jdalrymple commented 4 months ago

Darn, this is the library. In the library i assume the POST request is sending a body and not via query params. Ill fix this

jdalrymple commented 4 months ago

The docs make no indication of this though -_-

POST /projects/:id/merge_requests/:merge_request_iid/notes

jdalrymple commented 4 months ago

Ill file an issue with GL to figure out what should be happening here.

pvignau commented 4 months ago

Cheers mate, thanks for the quick answer !

The docs make no indication of this though -_-

POST /projects/:id/merge_requests/:merge_request_iid/notes

I found it on the PUT request doc : https://archives.docs.gitlab.com/16.5/ee/api/notes.html#modify-existing-merge-request-note

jdalrymple commented 4 months ago

Yea, there are definitely some inconsistencies. The example request for the modify call has it as a query param, but the upper docs make no indication that it should be a query param